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