Re: [PATCH v3] f2fs: add symbolic link to kobject in sysfs

From: Greg KH <gregkh@linuxfoundation.org>
To: Daeho Jeong <daeho43@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
	Daeho Jeong <daehojeong@google.com>
Subject: Re: [PATCH v3] f2fs: add symbolic link to kobject in sysfs
Date: Wed, 8 Jul 2020 12:05:27 +0200	[thread overview]
Message-ID: <20200708100527.GA448589@kroah.com> (raw)
In-Reply-To: <CACOAw_yweR--34vBXBV07xEGxGhO2r9o_XYVw6h9dMP=C6zp5Q@mail.gmail.com>

On Mon, Jul 06, 2020 at 08:47:07AM +0900, Daeho Jeong wrote:
> > No Documentation/ABI/ entry for your new sysfs file/link?
> 
> This is for adding a symbolic link to a pre-existed
> /sys/fs/f2fs/<disk> directory and it means /sys/fs/f2fs/<mount> points
> to /sys/fs/f2fs/<disk>. I already added the description of this in
> Documentation/filesystems/f2fs.rst.

Ok, but that's not the standard location for sysfs file documentation.

> > And what does this help with?
> 
> Some system daemons in Android access with the pre-defined sysfs entry
> name in the json file. So whenever the project changes and the
> partition layout is changed, we have to follow up the changes and
> modify /sys/fs/f2fs/<disk> name in the json file accordingly.

That's what partition names are for, you should never depend on a
"random number".

> This will help them access all the f2fs sysfs entries consistently
> without requiring to know those changes.

No, please use a partition name, that is the only way to always know you
are mounting the correct partition.  You have created a random number
here that might, or might not, change between boots depending on the
order of the filesystem being mounted.  It is not persistant or
deterministic at all, please do not treat it as such.

> > If it's really needed, why don't we do this for all filesystem types?
> 
> This is for the daemon to change the mode of only F2FS with the power
> hint of Android.

Again, the point is that a filesystem type is not unique, this, if
really really needed, should be an attribute for ALL filesystem types,
f2fs is not special here at all.

Please do not rely on this number ever being the same across boots,
because your code is such that you can not guarantee that.

And again, if you really want to know the partition you are mounting
really is the partition you think you are mounting, use the partition
label name, that is what it is there for, and is why we have been
relying on that for decades.  A new special per-filesystem-attribute
that is semi-random is not the correct solution for the problem you are
describing as far as I can determine.

thanks,

greg k-h