On Fri, Jul 04, 2025 at 11:33:09AM -0700, Justin Tee wrote:
> > Sure, but I'd rather do that as a followup.
> Yeup, that’s fine.
>
> > Speaking of other fun followups
> > in the same area: no point storing most of those dentries; debugfs_remove()
> > takes the entire subtree out, no need to remove them one-by-one and that
> > was the only use they were left... Something along the lines of
> > diff below:
> Very cool, thanks! We’ll take that diff too (:
>
> Also, may we actually move this enum declaration to lpfc_debugfs.h
> instead of lpfc_debugfs.c?
> enum {
> writeGuard = 1,
> writeApp,
> writeRef,
> readGuard,
> readApp,
> readRef,
> InjErrLBA,
> InjErrNPortID,
> InjErrWWPN,
> };
Sure, no problem... Your driver, your preferences - it's not as if
that had any impact outside.
While we are at it, handling of debugfs_vport_count looks fishy -
it's easier to spot with aforementioned delta applied.
if (vport->vport_debugfs_root) {
debugfs_remove(vport->vport_debugfs_root); /* vportX */
vport->vport_debugfs_root = NULL;
atomic_dec(&phba->debugfs_vport_count);
}
if (atomic_read(&phba->debugfs_vport_count) == 0) {
...
}
return;
is OK only if all updates of that thing are externally serialized.
If they are, we don't need atomic; if they are not, this separation
of decrement and test is racy.
Note that if you do *not* have external serialization, there might
be another problem if you have the last vport removal overlap
with addition of another vport. Or, for that matter, if removal
of the last vport on the last HBA overlaps with addition of a new
HBA...
Unless something has drastically changed, binding/unbinding of
a device to driver should be serialized by drivers/base/* logics;
no idea about the vports side of things...
