On Thu, 2024-11-07 at 10:40 +0100, Jan Kara wrote: > On Wed 06-11-24 14:53:06, Jeff Layton wrote: > > /proc/self/mountinfo displays the devicename for the mount, but > > statmount() doesn't yet have a way to return it. Add a new > > STATMOUNT_MNT_DEVNAME flag, claim the 32-bit __spare1 field to hold the > > offset into the str[] array. STATMOUNT_MNT_DEVNAME will only be set in > > the return mask if there is a device string. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > Just one question below: > > > @@ -5078,6 +5091,12 @@ static int statmount_string(struct kstatmount *s, u64 flag) > > if (seq->count == sm->fs_subtype) > > return 0; > > break; > > + case STATMOUNT_MNT_DEVNAME: > > + sm->mnt_devname = seq->count; > > + ret = statmount_mnt_devname(s, seq); > > + if (seq->count == sm->mnt_devname) > > Why this odd check? Why don't you rather do: > if (ret) > ? > statmount_mnt_devname() can return without emitting anything to the seq if ->show_devname and r->mnt_devname are both NULL. In that case, we don't want statmount_string() to return an error, but we also don't want to do any further manipulation of the seq. So, the above handles either the case where show_devname returned an error and the case where there was nothing to emit. I did consider having statmount_mnt_devname() return -ENOBUFS if there was nothing to emit, and then handle that in the caller, but checking to see whether the seq has advanced seemed like a cleaner solution. I can add a comment to that effect since it does look a bit confusing. > > + return ret; > > + break; > > default: > > WARN_ON_ONCE(true); > > return -EINVAL; > > Honza > -- Jeff Layton <jlayton@xxxxxxxxxx>
