linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Justin Tee <justintee8345@gmail.com>
Cc: linux-fsdevel@vger.kernel.org,
	James Smart <james.smart@broadcom.com>,
	Justin Tee <justin.tee@broadcom.com>,
	Greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons
Date: Fri, 4 Jul 2025 21:10:27 +0100	[thread overview]
Message-ID: <20250704201027.GS1880847@ZenIV> (raw)
In-Reply-To: <CABPRKS89iGUC5nih40yc9uRMkkfjZUafAN59WQBzpGC3vK_MkQ@mail.gmail.com>

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...

  reply	other threads:[~2025-07-04 20:10 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-02 21:13 [PATCHES] assorted debugfs stuff Al Viro
2025-07-02 21:14 ` [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops Al Viro
2025-07-02 21:15   ` [PATCH 02/11] hfi1: get rid of redundant debugfs_file_{get,put}() Al Viro
2025-07-02 21:16   ` [PATCH 03/11] regmap: " Al Viro
2025-07-03 11:10     ` Mark Brown
2025-07-02 21:16   ` [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}() Al Viro
2025-07-02 21:24     ` Luck, Tony
2025-07-02 23:45       ` Reinette Chatre
2025-07-03  0:24         ` Al Viro
2025-07-03  5:40           ` Greg Kroah-Hartman
2025-07-03 15:10             ` Reinette Chatre
2025-07-02 21:17   ` [PATCH 05/11] vmscan: don't bother with debugfs_real_fops() Al Viro
2025-07-03 11:51     ` Matthew Wilcox
2025-07-04  4:07       ` Al Viro
2025-07-02 21:22   ` [PATCH 06/11] netronome: " Al Viro
2025-07-03  7:11     ` Louis Peens
2025-07-02 21:24   ` [PATCH 07/11] debugfs: split short and full proxy wrappers, kill debugfs_real_fops() Al Viro
2025-07-02 21:25   ` [PATCH 08/11] fix tt_command_write() Al Viro
2025-07-03 11:14     ` Rafael J. Wysocki
2025-07-02 21:26   ` [PATCH 09/11] debugfs_get_aux(): allow storing non-const void * Al Viro
2025-07-02 21:28   ` [PATCH 10/11] blk-mq-debugfs: use debugfs_get_aux() Al Viro
2025-07-02 21:29   ` [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons Al Viro
     [not found]     ` <b3ff59d4-c6c3-4b48-97e3-d32e98c12de7@broadcom.com>
     [not found]       ` <CAAmqgVMmgW4PWy289P4a8N0FSZA+cHybNkKbLzFx_qBQtu8ZHA@mail.gmail.com>
2025-07-03 20:37         ` Justin Tee
2025-07-04  4:25           ` Al Viro
2025-07-04 18:33             ` Justin Tee
2025-07-04 20:10               ` Al Viro [this message]
2025-07-07 19:29                 ` Justin Tee
2025-07-08  2:57                   ` Al Viro
2025-07-08 17:17                     ` Justin Tee
2025-07-02 23:19   ` (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops Jens Axboe
2025-07-03  0:23     ` Al Viro
2025-07-03  0:56       ` Jens Axboe
2025-07-03 11:37       ` Greg Kroah-Hartman
2025-07-03 15:10         ` Al Viro
2025-07-04 15:34   ` Mark Brown
2025-07-09 11:35 ` [PATCHES] assorted debugfs stuff Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250704201027.GS1880847@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=james.smart@broadcom.com \
    --cc=justin.tee@broadcom.com \
    --cc=justintee8345@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).