Re: [PATCH 39/45] libxfs: remove pointless *XFS_MOUNT* flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 27, 2022 at 05:03:22PM -0600, Eric Sandeen wrote:
> On 1/19/22 6:20 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > 
> > Get rid of these flags and the m_flags field, since none of them do
> > anything anymore.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> 
> ...
> 
> > diff --git a/libxfs/init.c b/libxfs/init.c
> > index e9235a35..093ce878 100644
> > --- a/libxfs/init.c
> > +++ b/libxfs/init.c
> > @@ -540,13 +540,10 @@ xfs_set_inode_alloc(
> >   	 * sufficiently large, set XFS_MOUNT_32BITINODES if we must alter
> >   	 * the allocator to accommodate the request.
> >   	 */
> > -	if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) && ino > XFS_MAXINUMBER_32) {
> > +	if (ino > XFS_MAXINUMBER_32)
> >   		xfs_set_inode32(mp);
> > -		mp->m_flags |= XFS_MOUNT_32BITINODES;
> > -	} else {
> > +	else
> >   		xfs_clear_inode32(mp);
> > -		mp->m_flags &= ~XFS_MOUNT_32BITINODES;
> > -	}
> 
> Hm, so this just removes the "XFS_MOUNT_SMALL_INUMS" test. In the last
> release, nothing ever set this flag in userspace, so the first part of
> the conditional was always false, so we always cleared the 32bitinode
> setting.
> 
> So I think this is a change in behavior, and if we get a request for a
> large inode, we'll enable inode32, at least for this session?
> 
> But maybe that's ok, since there is no "inode32 mount option" in
> userspace, and maybe we *shouldn't* be allocating 64-bit inodes in
> userspace?  <thinking>
> 
> <thinking some more>
> 
> I'll get back to this :)

Ooh, good catch.  MOUNT_SMALL_INUMS was zero, so the first part of the
if test was alway zero, which means that we always call the else clause.
IOWs, inode64 should always be enabled, not inode32.

Maybe there's an argument for only using inode32 mode in userspace
(though userspace never adds user-visible files) so that a system that
has inode32 in fstab will never ever have big inodes?

Dunno, but that's a separate patch if people really want that.

I'll respin this patch with fixed logic.  I'll also fix the comments.

--D

> -Eric
> 
> 



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux