On Wed, Aug 23, 2023 at 5:02 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Aug 22, 2023 at 04:20:06PM -0400, Alistair Francis wrote: > > On Sat, Aug 19, 2023 at 6:57 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Thu, Aug 17, 2023 at 07:58:09PM -0400, Alistair Francis wrote: > > > > The documentation for sysfs_merge_group() specifically says > > > > > > > > This function returns an error if the group doesn't exist or any of the > > > > files already exist in that group, in which case none of the new files > > > > are created. > > > > > > > > So just not adding the group if it's empty doesn't work, at least not > > > > with the code we currently have. The code can be changed to support > > > > this, but it is difficult to determine how this will affect existing use > > > > cases. > > > > > > Did you try? I'd really really really prefer we do it this way as it's > > > much simpler overall to have the sysfs core "do the right thing > > > automatically" than to force each and every bus/subsystem to have to > > > manually add this type of attribute. > > > > > > Also, on a personal level, I want this function to work as it will allow > > > us to remove some code in some busses that don't really need to be there > > > (dynamic creation of some device attributes), which will then also allow > > > me to remove some apis in the driver core that should not be used at all > > > anymore (but keep creeping back in as there is still a few users.) > > > > > > So I'll be selfish here and say "please try to get my proposed change to > > > work, it's really the correct thing to do here." > > > > I did try! > > > > This is an attempt: > > https://github.com/alistair23/linux/commit/56b55756a2d7a66f7b6eb8a5692b1b5e2f81a9a9 > > > > It is based on your original patch with a bunch of: > > > > if (!parent) { > > parent = kernfs_create_dir_ns(kobj->sd, grp->name, > > S_IRWXU | S_IRUGO | S_IXUGO, > > uid, gid, kobj, NULL); > > ... > > } > > > > > > added throughout the code base. > > > > Very basic testing shows that it does what I need it to do and I don't > > see any kernel oops on boot. > > Nice! > > Mind if I take it and clean it up a bit and test with it here? Again, I > need this functionality for other stuff as well, so getting it merged > for your feature is fine with me. Sure! Go ahead. Sorry I was travelling last week. > > > I prefer the approach I have in this mailing list patch. But if you > > like the commit mentioned above I can tidy and clean it up and then > > use that instead > > I would rather do it this way. I can work on this on Friday if you want > me to. Yeah, that's fine with me. If you aren't able to let me know and I can finish up the patch and send it with this series Alistair > > thanks, > > greg k-h
