Re: [PATCH v8 12/16] xfs: dont set sb in xfs_mount_alloc()

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

 



On Fri, 2019-11-01 at 13:15 -0700, Darrick J. Wong wrote:
> On Fri, Nov 01, 2019 at 03:51:06PM +0800, Ian Kent wrote:
> > When changing to use the new mount api the super block won't be
> > available when the xfs_mount struct is allocated so move setting
> > the
> > super block in xfs_mount to xfs_fs_fill_super().
> > 
> > Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
> > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> > ---
> >  fs/xfs/xfs_super.c |    7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 4b570ba3456a..62dfc678c415 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1560,8 +1560,7 @@ xfs_destroy_percpu_counters(
> >  }
> >  
> >  static struct xfs_mount *
> > -xfs_mount_alloc(
> > -	struct super_block	*sb)
> > +xfs_mount_alloc(void)
> >  {
> >  	struct xfs_mount	*mp;
> >  
> > @@ -1569,7 +1568,6 @@ xfs_mount_alloc(
> >  	if (!mp)
> >  		return NULL;
> >  
> > -	mp->m_super = sb;
> 
> Just out of curiosity, is there any place where we need m_super in
> between here...
> 
> >  	spin_lock_init(&mp->m_sb_lock);
> >  	spin_lock_init(&mp->m_agirotor_lock);
> >  	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
> > @@ -1605,9 +1603,10 @@ xfs_fs_fill_super(
> >  	 * allocate mp and do all low-level struct initializations
> > before we
> >  	 * attach it to the super
> >  	 */
> > -	mp = xfs_mount_alloc(sb);
> > +	mp = xfs_mount_alloc();
> >  	if (!mp)
> >  		goto out;
> > +	mp->m_super = sb;
> 
> ...and here?  For example, logging errors?  AFAICT the only thing
> that
> goes on between these two points is option parsing, right?  (And the
> parsing has its own prefixed logging, etc.)

Yes, only option parsing is going on between these two points.

And, for now, the error reporting is caught by the VFS.

There is one location in xfs_fc_parse_param() where an xfs log
message could be emitted although it's never reached (because of
the return if the fs_parse() call fails).

If log messages were issued in between these two points the consequence
is a missing block device name in the message. You remember, a check on
mp->m_super was added to __xfs_printk() to cover this case when struct
xfs_mount field m_fsname was eliminated.

This potential lack of device name in log messages is a problem I can't
fix because the block device isn't obtained until after parameter
parsing, just before the super block is acquired. Changing that in the
VFS would be quite significant so I'm stuck!

> 
> --D
> 
> >  	sb->s_fs_info = mp;
> >  
> >  	error = xfs_parseargs(mp, (char *)data);
> > 




[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