Received-SPF: pass (google.com: domain of linux-kernel+bounces-40540-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 26 Jan 2024 11:07:13 -0800
From: Greg KH <gregkh@linuxfoundation.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Alistair Francis <alistair23@gmail.com>, bhelgaas@google.com,
	linux-pci@vger.kernel.org, Jonathan.Cameron@huawei.com,
	lukas@wunner.de, alex.williamson@redhat.com,
	christian.koenig@amd.com, kch@nvidia.com, logang@deltatee.com,
	linux-kernel@vger.kernel.org, chaitanyak@nvidia.com,
	rdunlap@infradead.org, Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH v6 2/3] sysfs: Add a attr_is_visible function to
 attribute_group
Message-ID: <2024012631-removed-stuffed-ecc0@gregkh>
References: <2023083139-underling-amuser-772e@gregkh>
 <2023090142-circling-probably-7828@gregkh>
 <2023100539-playgroup-stoppable-d5d4@gregkh>
 <CAKmqyKOgej1jiiHsoLuDKXwdLDGa4njrndu6c1=bxnqk2NM58Q@mail.gmail.com>
 <2023101113-swimwear-squealer-464c@gregkh>
 <CAKmqyKMX3HDphrWHYcdnLEjMwe1pCROcPNZchPonhsLOq=FoHw@mail.gmail.com>
 <CAKmqyKOOSBF7qDpqAp6nn3+3wAnaGmqu88Fk3KY58fmgQ-44Jw@mail.gmail.com>
 <2024012321-envious-procedure-4a58@gregkh>
 <65b1739b2c789_37ad294f5@dwillia2-xfh.jf.intel.com.notmuch>
 <65b400bf65c33_51c7f294c5@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65b400bf65c33_51c7f294c5@dwillia2-mobl3.amr.corp.intel.com.notmuch>

On Fri, Jan 26, 2024 at 10:58:07AM -0800, Dan Williams wrote:
> Dan Williams wrote:
> > Greg KH wrote:
> > [..]
> > > > 
> > > > Hey Greg,
> > > > 
> > > > I wanted to follow up on this and see if you are able to provide more
> > > > details for reproducing or if you are able to look into it?
> > > 
> > > Last I tried this, it still crashed and would not boot either on my
> > > laptop or my workstation.  I don't know how it is working properly for
> > > you, what systems have you tried it on?
> > > 
> > > I'm not going to be able to look at this for many weeks due to
> > > conference stuff, so if you want to take the series and test it and
> > > hopefully catch my error, that would be great, I'd love to move forward
> > > and get this merged someday.
> > 
> > I mentioned to Lukas that I was working on a "sysfs group visibility"
> > patch and he pointed me to this thread. I will note that I tried to make
> > the "hide group if all attributes are invisible" approach work, but
> > reverted to a "new is_group_visible() callback" approach. I did read
> > through the thread and try to improve the argument in the changelog
> > accordingly.
> > 
> > I do admit to liking the cleanliness (not touching 'struct
> > attribute_group') of the "hide if no visible attribute" approch, but see
> > the criticism of that alternative below, and let me know if it is
> > convincing. I tested it locally with the following hack to make the
> > group disappear every other sysfs_update_group() event:
> 
> Hey Greg,
> 
> Ignore this version:
> 
> ---
> From: Dan Williams <dan.j.williams@intel.com>
> Date: Tue, 23 Jan 2024 20:20:39 -0800
> Subject: [PATCH] sysfs: Introduce is_group_visible() for attribute_groups
> ---
> 
> I am going back to your approach without a new callback, and some fixups
> to avoid unintended directory removal. I will post that shortly with its
> consumer.

Ignore it?  I was just about to write an email that said "maybe this is
the right way forward" :)

What happened to cause it to not be ok?  And if you can find the bug in
the posted patch here, that would be great as well.

thanks,

greg k-h