on 2022/4/26 1:33, Matthew Wilcox wrote: > On Mon, Apr 25, 2022 at 01:29:47PM +0200, Christian Brauner wrote: >> On Mon, Apr 25, 2022 at 03:08:36AM +0000, xuyang2018.jy@xxxxxxxxxxx wrote: >>> on 2022/4/25 10:45, Matthew Wilcox wrote: >>>> On Mon, Apr 25, 2022 at 11:09:38AM +0800, Yang Xu wrote: >>>>> This has no functional change. Just create and export inode_sgid_strip >>>>> api for the subsequent patch. This function is used to strip inode's >>>>> S_ISGID mode when init a new inode. >>>> >>>> Why would you call this inode_sgid_strip() instead of >>>> inode_strip_sgid()? >>> >>> Because I treated "inode sgid(inode's sgid)" as a whole. >>> >>> inode_strip_sgid sounds also ok, but now seems strip_inode_sgid seem >>> more clear because we strip inode sgid depend on not only inode's >>> condition but also depend on parent directory's condition. >>> >>> What do you think about this? >>> >>> ps: I can aceept the above several way, so if you insist, I can change >>> it to inode_strip_sgid. >> >> I agree with Willy. I think inode_strip_sgid() is better. It'll be in >> good company as<object>_<verb>_<what?> is pretty common: >> >> inode_update_atime() >> inode_init_once() >> inode_init_owner() >> inode_init_early() >> inode_add_lru() >> inode_needs_sync() >> inode_set_flags() >> >> Maybe mode_remove_sgid() is even better because it makes it clear that >> the change happens to @mode and not @dir. But I'm fine with >> inode_strip_sgid() or inode_remove_sgid() too. > > Oh! Yes, mode_strip_sgid() is better. We're operating on the mode, > not the inode. OK, I will use mode_strip_sgid().