[v4,0/6] xfs: some xfs_dialloc() cleanup

Message

Gao Xiang Dec. 8, 2020, 12:19 p.m. UTC

Hi folks,

This is v4 of the following patchset
https://lore.kernel.org/r/[e-mail]
, which tends to simplify xfs_dialloc() logic.

This version includes Dave's original patch
https://lore.kernel.org/r/[e-mail]

to avoid the original double call of xfs_dialloc() and confusing
ialloc_context with some split in order for better review and minor
modification (e.g. ino isn't passed in xfs_ialloc()).

I'm not quite sure what's messy ENOSPC mentioned in the original
patch since return 0 and *ipp = NULL in xfs_dir_ialloc() would cause
NULL-dereference in its callers, so I leave this part alone, although
I think noroom has no use at all (it can be cleaned up as well with
a following patch) (at a glance, the final shape looks almost ok...)

I dropped [PATCH v1 3/3] since xfs_ialloc_select_ag() already looks
simple enough (comments about this are welcome... I can re-add this
if needed.)

I'm re-runing xfstests -g auto with the whole series for now since
a bit left behind this time, hopefully no extra issues here. I've
dropped all RVBs from "[PATCH v4 3/6]" since some more modification
here...

Thanks for your time.

changes since v3 (Christoph):
 - (2/6) get rid of a brace;
 - (2/6) keeping xfs_buf_relse() in the callers;
 - (2/6) get rid of "XXX:" comment;
 - (3/6) rename xfs_ialloc() to xfs_init_new_inode();
 - (3/6) rename pino to parent_ino;
 - (3/6) convert the return type from int to struct xfs_inode *
         (so I dropped all RVBs of this patch, since more modification
          here....)
 - (4/6) refine a comment;
 - (5/6) add a found_ag lebel to have a central place for the
         sucessful return, since there are somewhat too many labels here,
         I didn't add another error lebel to make *IO_agbp = NULL and
         return error;
 - (6/6) drop the else after the break.

changes since v2:
 - use struct xfs_dquot_acct * instead of void * on dqinfo (Darrick);
 - rename xfs_ialloc() to xfs_dir_ialloc_init() (Dave);
 - fix a temporary state compile error due to (ialloc_context ->
   &ialloc_context) on [PATCH v2 3/6];
 - collect more RVBs from the reply of v2;
 - Cc Eric to confirm dqinfo;

changes since v1:
 - add Dave's patch with spilt and minor update;
 - update comments above xfs_ialloc_ag_alloc() suggested by Darrick;
 - collect RVBs to
    "xfs: convert noroom, okalloc in xfs_dialloc() to bool"
    "xfs: kill ialloced in xfs_dialloc()"
   since no real logic changes ("(!error)" to "(error==0)" suggested
   by Darrick has been updated).

Thanks,
Gao Xiang


Dave Chinner (4):
  xfs: introduce xfs_dialloc_roll()
  xfs: move on-disk inode allocation out of xfs_ialloc()
  xfs: move xfs_dialloc_roll() into xfs_dialloc()
  xfs: spilt xfs_dialloc() into 2 functions

Gao Xiang (2):
  xfs: convert noroom, okalloc in xfs_dialloc() to bool
  xfs: kill ialloced in xfs_dialloc()

 fs/xfs/libxfs/xfs_ialloc.c | 174 +++++++++++++------------
 fs/xfs/libxfs/xfs_ialloc.h |  36 +++---
 fs/xfs/xfs_inode.c         | 259 +++++++++----------------------------
 fs/xfs/xfs_inode.h         |   6 +-
 fs/xfs/xfs_qm.c            |  27 ++--
 fs/xfs/xfs_symlink.c       |   8 +-
 6 files changed, 197 insertions(+), 313 deletions(-)