On Thu, Mar 23 2023 at 3:51P -0400, Joe Thornber <thorn...@redhat.com> wrote:
> Mike, > > I'm Nacking this patch here. You've taken the comment from my latest > patch, and attached it to something else. I don't know if you've taken an > older patch and updated it, or taken the latest patch and downgraded it. I lean on git to tell me if code is identical. Since we're now clearly keeping score here. I used git to compare what I had here (with various small whitespace tweaks and other nits): https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=2023-03-01-thin-concurrency-8 with your -8 code: https://github.com/jthornber/linux/tree/2023-03-21-thin-concurrency-8 I had to deal with the problem that you didn't use my tree you rebased so I had to reconcile not losing my handful of fluff. I leaned on the fact that your -7 and -8 were identical in code: git diff ejt/2023-02-28-thin-concurrency-7 ejt/2023-03-21-thin-concurrency-8 -- drivers/md/dm-bufio.c I could've just generated my fluff and folded it into your new clean -8. But I went the other way around, instead I picked up your other incremental changes from -7 and folded them into what I had. No real reason, that's just how I did it. > For instance the latest patch uses the name 'struct dm_buffer_cache', but > it looks like you've renamed to 'struct buffer_cache' and then posted > another patch in the series that renames back to dm_buffer_cache. Right, I was cherry-picking your incremantal changes your staged in -7 before your clean patch rollup in your -8. How the struct rename occurred to the code didn't much matter to me, so long as the change was made. But yeah I should've just folded it. Not a big deal. > I also see the comment change: > > original "A refcount_t for hold_count does not work." which has changed to > "NOTE: attempting to use refcount_t instead of atomic_t caused crashes!". > Talk about over dramatisation (If the hold_count is incorrect we detect and > BUG). Yes, but I was aiming to leave breadcrumbs if/when someone might think to retry using refcount_t to see why it "does not work". I was trying to save future me or you or someone else some time. By making it clear: there will be dragons. But in doing so I unwittingly already unleashed different dragons ;) > I can't stand by my code if I don't know what's being sent up in my name. > > Please use the patchset from this branch: > https://github.com/jthornber/linux/tree/2023-03-21-thin-concurrency-8 We've resolved this now interactively in chat, but I had to reply on dm-devel to let others know. Again, on a code level what I posted to dm-devel reflects what was staged in 2023-03-21-thin-concurrency-8: For me this yields my fluff I was referring to above: git diff ejt/2023-03-21-thin-concurrency-8 dm/dm-6.4 -- drivers/md/dm-bufio.c Biggest difference being dm_buffer struct member reordering (which I had discussed with you). For the benefit of others, I since dropped patch 1 and patch 2 (max_discard_granularity and bio-prison-v1 patch) but pulled them to the HEAD of this new development branch (that has extra debugging): https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=dm-6.4 I'll go back to bed now, hopefully you'll have this discard issue fixed in a couple hours when I return (as we discussed). ;) Mike -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
