On Thu, Oct 03, 2019 at 06:25:18PM +0800, Ian Kent wrote: > This patch series add support to xfs for the new kernel mount API > as described in the LWN article at https://lwn.net/Articles/780267/. > > In the article there's a lengthy description of the reasons for > adopting the API and problems expected to be resolved by using it. > > The series has been applied to the repository located at > git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git, and built and > some simple tests run on it along with the generic xfstests. > I'm not sure that we have any focused mount option testing in fstests. It looks like we have various remount tests and such to cover corner cases and/or specific bugs, but nothing to make sure various options continue to work or otherwise fail gracefully. Do you have any plans to add such a test to help verify this work? Brian > Other things that continue to cause me concern: > > - Message logging. > There is error logging done in the VFS by the mount-api code, some > is VFS specific while some is file system specific. This can lead > to duplicated and sometimes inconsistent logging. > > The mount-api feature of saving error message text to the mount > context for later retrieval by fsopen()/fsconfig()/fsmount() users > is the reason the mount-api log macros are present. But, at the > moment (last time I looked), these macros will either log the > error message or save it to the mount context. There's not yet > a way to modify this behaviour so it which can lead to messages, > possibly needed for debug purposes, not being sent to the kernel > log. There's also the pr_xxx() log functions (not a problem for > xfs AFAICS) that aren't aware of the mount context at all. > > In the xfs patches I've used the same method that is used in > gfs2 and was suggested by Al Viro (essentially return the error > if fs_parse() returns one) except that I've also not used the > mount api log macros to minimise the possibility of lost log > messages. > > This isn't the best so follow up patches for RFC (with a > slightly wider audience) will be needed to try and improve > this aspect of the mount api. > > Changes for v4: > - changed xfs_fill_super() cleanup back to what it was in v2, until > I can work out what's causing the problem had previously seen (I can't > reproduce it myself), since it looks like it was right from the start. > - use get_tree_bdev() instead of vfs_get_block_super() in xfs_get_tree() > as requested by Al Viro. > - removed redundant initialisation in xfs_fs_fill_super(). > - fix long line in xfs_validate_params(). > - no need to validate if parameter parsing fails, just return the error. > - summarise reconfigure comment about option handling, transfer bulk > of comment to commit log message. > - use minimal change in xfs_parse_param(), deffer discussion of mount > api logging improvements until later and with a wider audience. > > Changes for v3: > - fix struct xfs_fs_context initialisation in xfs_parseargs(). > - move call to xfs_validate_params() below label "done". > - if allocation of xfs_mount fails return ENOMEM immediately. > - remove erroneous kfree(mp) in xfs_fill_super(). > - move the removal of xfs_fs_remount() and xfs_test_remount_options() > to the switch to mount api patch. > - retain original usage of distinct <option>, no<option> usage. > - fix line length and a white space problem in xfs_parseargs(). > - defer introduction of struct fs_context_operations until mount > api implementation. > - don't use a new line for the void parameter of xfs_mount_alloc(). > - check for -ENOPARAM in xfs_parse_param() to report invalid options > using the options switch (to avoid double entry log messages). > - remove obsolete mount option biosize. > - try and make comment in xfs_fc_free() more understandable. > > Changes for v2: > - changed .name to uppercase in fs_parameter_description to ensure > consistent error log messages between the vfs parser and the xfs > parameter parser. > - clarify comment above xfs_parse_param() about when possibly > allocated mp->m_logname or mp->m_rtname are freed. > - factor out xfs_remount_rw() and xfs_remount_ro() from xfs_remount(). > - changed xfs_mount_alloc() to not set super block in xfs_mount so it > can be re-used when switching to the mount-api. > - fixed don't check for NULL when calling kfree() in xfs_fc_free(). > - refactored xfs_parseargs() in an attempt to highlight the code > that actually changes in converting to use the new mount api. > - dropped xfs-mount-api-rename-xfs_fill_super.patch, it didn't seem > necessary. > - move comment about remount difficulties above xfs_reconfigure() > and increase line length to try and make the comment manageable. > > Al Viro has sent a pull request to Linus for the patch containing > get_tree_bdev() recently and I think there's a small problem with > that patch too so there will be conflicts with merging this series > without dropping the first two patches of the series. > > --- > > David Howells (1): > vfs: Create fs_context-aware mount_bdev() replacement > > Ian Kent (16): > vfs: add missing blkdev_put() in get_tree_bdev() > xfs: remove very old mount option > xfs: mount-api - add fs parameter description > xfs: mount-api - refactor suffix_kstrtoint() > xfs: mount-api - refactor xfs_parseags() > xfs: mount-api - make xfs_parse_param() take context .parse_param() args > xfs: mount-api - move xfs_parseargs() validation to a helper > xfs: mount-api - refactor xfs_fs_fill_super() > xfs: mount-api - add xfs_get_tree() > xfs: mount-api - add xfs_remount_rw() helper > xfs: mount-api - add xfs_remount_ro() helper > xfs: mount api - add xfs_reconfigure() > xfs: mount-api - add xfs_fc_free() > xfs: mount-api - dont set sb in xfs_mount_alloc() > xfs: mount-api - switch to new mount-api > xfs: mount-api - remove remaining legacy mount code > > > fs/super.c | 97 +++++ > fs/xfs/xfs_super.c | 939 +++++++++++++++++++++++--------------------- > include/linux/fs_context.h | 5 > 3 files changed, 600 insertions(+), 441 deletions(-) > > -- > Ian
