Thread (19 messages) 19 messages, 4 authors, 2018-01-24

Re: [PATCH 0/4] Buffer's log item refactoring

From: Carlos Maiolino <hidden>
Date: 2018-01-23 08:39:50

On Mon, Jan 22, 2018 at 01:42:26PM -0800, Darrick J. Wong wrote:
On Mon, Jan 22, 2018 at 12:39:50PM +0100, Carlos Maiolino wrote:
quoted
On Sat, Jan 20, 2018 at 03:54:37PM +1100, Dave Chinner wrote:
quoted
On Fri, Jan 19, 2018 at 01:43:41PM -0800, Darrick J. Wong wrote:
quoted
On Fri, Jan 19, 2018 at 03:08:43PM +0100, Carlos Maiolino wrote:
quoted
Hi,

A few time ago Christoph suggested to use list_head API to handle buffer's
log_item_list.

This patchset aims to split the current bp->b_fspriv field into a specific field
to hold the xfs_buf_log_item, and another to hold the list of log items attached
to the buffer (3rd patch), and finally replace the singly linked list
implementation by the list_head infra-structure (4th patch).

The first two patches are just a typedef removal of xfs_buf_log_item_t and
xfs_buf_t, I did while studying how all the buffer I/O mechanism works, I
thought since we plan to get rid of the typedefs in future, this might be
useful.

I can rebase the 3rd and 4th patch on top of current xfs tree if the typedef
removal patches are useless, you guys call.
Typedef removal seems useful... is this series based atop current for-next?
It was rebased on xfs/for-next.
quoted
The reason we haven't done this sort of thing in the past is that it
causes problems for people with work under development that touches
the same code. Changes like this have knock-on effects, and they are
not necessary for the changes to the li_bio_list that the last two
patches provide.

e.g. This is going to cause me pain, because I've got lots of
modifications to xfs_buf related code pending in my dev tree. A
quick glance shows a >50% hunk conflict rate across all the changes
in my tree, and that's just the kernel side.  It's hard enough to
rebase deep patch stacks correctly without having to deal with the
noise introduced by this sort of tree-wide code cleanup....

The historic process is that we remove typedefs as we go, and when
there are relatively few of them left we can do a sweep. The
xfs_buf_log_item_t is at that threshold (only about 10 uses left).

However, there's ~150 xfs_buf_t references across the kernel code
base, and there's another 175 across the xfsprogs code base of which
only 40 are in the shared libxfs code (i.e. ~300 unique xfs_buf_t
references). IMO it's still used in far to much code to do a sweep
like this without making a lot of otherwise unnecessary work for
others...
Ok, so I'll drop the typedef patches, well, should I drop both of them or just
the xfs_buf_t? Since the xfs_buf_log_item_t has very few usages now.
I think it's fine to drop only the xfs_buf_t cleanup and resend with
the other cleanups.
Thanks, I'll do that and hopefully send the V2 today yet

Cheers

-- 
Carlos
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help