Message ID | r75jqqdjp24gikil2l26wwtxdxvqxpgfaixb2rqmuyzxnbhseq@6k34emck64hv (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [GIT,PULL] bcachefs changes for 6.11 | expand |
Hi Kent, On Sun, 14 Jul 2024 21:26:30 -0400 Kent Overstreet <kent.overstreet@linux.dev> wrote: > > Hi Linus - another opossum for the posse: > > The following changes since commit 0c3836482481200ead7b416ca80c68a29cfdaabd: > > Linux 6.10 (2024-07-14 15:43:32 -0700) > > are available in the Git repository at: > > https://evilpiepirate.org/git/bcachefs.git tags/bcachefs-2024-07-14 > > for you to fetch changes up to efb2018e4d238cc205690ac62c0917d60d291e66: > > bcachefs: Kill bch2_assert_btree_nodes_not_locked() (2024-07-14 19:59:12 -0400) We normally expect branches to not be rebased just before being sent to Linus for merging. See: Documentation/maintainer/rebasing-and-merging.rst
On Sun, 14 Jul 2024 at 18:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > Hi Linus - another opossum for the posse: (The kernel naming tends to be related to some random event, in this case we had a family of opossums under our shed for a couple of months) > bcachefs changes for 6.11-rc1 As Stephen pointed out, all of this seems to have been rebased basically as the merge window opened, so if it was in linux-next, I certainly can't easily validate it without having to compare patch ids etc. DON'T DO THIS. Also, the changes to outside fs/bcachefs had questions that weren't answered. So I'm just dropping this all for now. Linus
On Wed, Jul 17, 2024 at 11:53:04AM GMT, Linus Torvalds wrote: > On Sun, 14 Jul 2024 at 18:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > > > Hi Linus - another opossum for the posse: > > (The kernel naming tends to be related to some random event, in this > case we had a family of opossums under our shed for a couple of > months) Oh cute :) > > bcachefs changes for 6.11-rc1 > > As Stephen pointed out, all of this seems to have been rebased > basically as the merge window opened, so if it was in linux-next, I > certainly can't easily validate it without having to compare patch ids > etc. DON'T DO THIS. I had to give this some thought; the proximate cause was just fat fingering/old reflexes, but the real issue that's been causing conflicts is that I've got testers running my trees who very much /do/ need to be on the latest tagged release. And I can't just leave it for them to do a rebase/merge, because a) they don't do that, and b) then I'm looking at logs with commits I can't reference. So - here's how my branches are going to be from now on: As before: - bcachefs-testing: code goes here first, until it's passed the testing automation. Don't run this unless you're working with me on something. - for-next: the subset of bcachefs-testing that's believed to be stable - bcachefs-for-upstream: queue for next pull request, generally just hotfixes But my master branch (previously the same as for-next) will now be for-next merged with the latest tag from your tree, and I may do similarly for bcachefs-for-upstream if it's needed. As a bonus, this means the testing automation will now be automatically testing my branch + your latest; this would have caught the breakage from Christoph's FUA changes back in 6.7. > Also, the changes to outside fs/bcachefs had questions that weren't answered. Yeah, those comments should have been added. Waiman, how's this? -- >8 -- From 1d8cbc45ef1bab9be7119e0c5a6f8a05d5e2ca7d Mon Sep 17 00:00:00 2001 From: Kent Overstreet <kent.overstreet@linux.dev> Date: Thu, 18 Jul 2024 17:17:10 -0400 Subject: [PATCH] lockdep: Add comments for lockdep_set_no{validate,track}_class() Cc: Waiman Long <longman@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index b76f1bcd2f7f..bdfbdb210fd7 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -178,9 +178,22 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name, (lock)->dep_map.wait_type_outer, \ (lock)->dep_map.lock_type) +/** + * lockdep_set_novalidate_class: disable checking of lock ordering on a given + * lock + * + * Lockdep will still record that this lock has been taken, and print held + * instances when dumping locks + */ #define lockdep_set_novalidate_class(lock) \ lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock) +/** + * lockdep_set_notrack_class: disable lockdep tracking of a given lock entirely + * + * Bigger hammer than lockdep_set_novalidate_class: so far just for bcachefs, + * which takes more locks than lockdep is able to track (48). + */ #define lockdep_set_notrack_class(lock) \ lockdep_set_class_and_name(lock, &__lockdep_no_track__, #lock)
On Thu, Jul 18, 2024 at 05:20:54PM -0400, Kent Overstreet wrote: > From: Kent Overstreet <kent.overstreet@linux.dev> > Date: Thu, 18 Jul 2024 17:17:10 -0400 > Subject: [PATCH] lockdep: Add comments for lockdep_set_no{validate,track}_class() > > Cc: Waiman Long <longman@redhat.com> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index b76f1bcd2f7f..bdfbdb210fd7 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -178,9 +178,22 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name, > (lock)->dep_map.wait_type_outer, \ > (lock)->dep_map.lock_type) > > +/** > + * lockdep_set_novalidate_class: disable checking of lock ordering on a given > + * lock > + * > + * Lockdep will still record that this lock has been taken, and print held > + * instances when dumping locks > + */ Might want to run this through kernel-doc. I'm pretty sure it wants macro comments to be formatted like function comments. ie: /** * lockdep_set_novalidate_class - Disable lock order checks on this lock. * @lock: The lock to disable order checks for. * * Lockdep will still record that this lock has been taken, and print held * instances when dumping locks. */ > #define lockdep_set_novalidate_class(lock) \ > lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock)
On 7/18/24 17:20, Kent Overstreet wrote: > On Wed, Jul 17, 2024 at 11:53:04AM GMT, Linus Torvalds wrote: >> On Sun, 14 Jul 2024 at 18:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: >>> Hi Linus - another opossum for the posse: >> (The kernel naming tends to be related to some random event, in this >> case we had a family of opossums under our shed for a couple of >> months) > Oh cute :) > >>> bcachefs changes for 6.11-rc1 >> As Stephen pointed out, all of this seems to have been rebased >> basically as the merge window opened, so if it was in linux-next, I >> certainly can't easily validate it without having to compare patch ids >> etc. DON'T DO THIS. > I had to give this some thought; the proximate cause was just > fat fingering/old reflexes, but the real issue that's been causing > conflicts is that I've got testers running my trees who very much /do/ > need to be on the latest tagged release. > > And I can't just leave it for them to do a rebase/merge, because a) they > don't do that, and b) then I'm looking at logs with commits I can't > reference. > > So - here's how my branches are going to be from now on: > > As before: > > - bcachefs-testing: code goes here first, until it's passed the testing > automation. Don't run this unless you're working with me on something. > - for-next: the subset of bcachefs-testing that's believed to be stable > - bcachefs-for-upstream: queue for next pull request, generally just > hotfixes > > But my master branch (previously the same as for-next) will now be > for-next merged with the latest tag from your tree, and I may do > similarly for bcachefs-for-upstream if it's needed. > > As a bonus, this means the testing automation will now be automatically > testing my branch + your latest; this would have caught the breakage > from Christoph's FUA changes back in 6.7. > >> Also, the changes to outside fs/bcachefs had questions that weren't answered. > Yeah, those comments should have been added. Waiman, how's this? > > -- >8 -- > > From 1d8cbc45ef1bab9be7119e0c5a6f8a05d5e2ca7d Mon Sep 17 00:00:00 2001 > From: Kent Overstreet <kent.overstreet@linux.dev> > Date: Thu, 18 Jul 2024 17:17:10 -0400 > Subject: [PATCH] lockdep: Add comments for lockdep_set_no{validate,track}_class() > > Cc: Waiman Long <longman@redhat.com> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index b76f1bcd2f7f..bdfbdb210fd7 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -178,9 +178,22 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name, > (lock)->dep_map.wait_type_outer, \ > (lock)->dep_map.lock_type) > > +/** > + * lockdep_set_novalidate_class: disable checking of lock ordering on a given > + * lock > + * > + * Lockdep will still record that this lock has been taken, and print held > + * instances when dumping locks > + */ > #define lockdep_set_novalidate_class(lock) \ > lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock) > > +/** > + * lockdep_set_notrack_class: disable lockdep tracking of a given lock entirely > + * > + * Bigger hammer than lockdep_set_novalidate_class: so far just for bcachefs, > + * which takes more locks than lockdep is able to track (48). > + */ > #define lockdep_set_notrack_class(lock) \ > lockdep_set_class_and_name(lock, &__lockdep_no_track__, #lock) > > That should be good enough. Thanks, Longman
On 7/18/24 17:27, Matthew Wilcox wrote: > On Thu, Jul 18, 2024 at 05:20:54PM -0400, Kent Overstreet wrote: >> From: Kent Overstreet <kent.overstreet@linux.dev> >> Date: Thu, 18 Jul 2024 17:17:10 -0400 >> Subject: [PATCH] lockdep: Add comments for lockdep_set_no{validate,track}_class() >> >> Cc: Waiman Long <longman@redhat.com> >> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> >> >> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h >> index b76f1bcd2f7f..bdfbdb210fd7 100644 >> --- a/include/linux/lockdep.h >> +++ b/include/linux/lockdep.h >> @@ -178,9 +178,22 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name, >> (lock)->dep_map.wait_type_outer, \ >> (lock)->dep_map.lock_type) >> >> +/** >> + * lockdep_set_novalidate_class: disable checking of lock ordering on a given >> + * lock >> + * >> + * Lockdep will still record that this lock has been taken, and print held >> + * instances when dumping locks >> + */ > Might want to run this through kernel-doc. I'm pretty sure it wants > macro comments to be formatted like function comments. ie: > > /** > * lockdep_set_novalidate_class - Disable lock order checks on this lock. > * @lock: The lock to disable order checks for. > * > * Lockdep will still record that this lock has been taken, and print held > * instances when dumping locks. > */ > >> #define lockdep_set_novalidate_class(lock) \ >> lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock) Yes, that is true. It is not in the proper kernel-doc format. Either use the proper format or use a single "*" instead. Cheers, Longman
On Thu, 18 Jul 2024 at 14:21, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > But my master branch (previously the same as for-next) will now be > for-next merged with the latest tag from your tree, and I may do > similarly for bcachefs-for-upstream if it's needed. No. No back-merges. We actually have documentation about this, so I won't repeat the hundreds of emails I've sent out: Documentation/maintainer/rebasing-and-merging.rst But the gist of it (well, one of them) is that you should keep your branch *YOUR* branch, not think that you should merge in other peoples work into it (or rebase it on top of random work by other developers). There are valid reasons to rebase, but they are balanced against some VERY valid reasons not to do it, so if you have to rebase, make sure those reasons really outweigh the reasons not to. And don't do it just before a pull request - and if there is some catastrophic event that caused a recent rebase, let me know in the pull request. > As a bonus, this means the testing automation will now be automatically > testing my branch + your latest No. That's what linux-next is about - it does integration testing. Your development branch IS NOT FOR TESTING OTHER RANDOM THINGS. You are actively making things worse if you do: you should worry about YOUR code, not about all the random other things going on. Now, if you want to do _additional_ testing along the lines of "what happens with my branch and Linus' latest" then that is ok - but that should be something you see as completely separate from testing your own work. IOW, you can certainly wear "multiple hats" - your bcachefs developer hat that worries about your bcachefs branch, and if you want to *also* do some integration testing, go right ahead and have another "integration testing branch" that you then test separately. But I don't want your integration path. When I get a bcachefs pull, I want the *bcachefs* side to be solid and tested. Not something else. So by all means keep multiple branches for different reasons. If you think you have users that need to test some integration branch (which honestly sounds like a horrible idea, but you do you), make them a branch too if you really want to. But again, that has NOTHING to do with bcachefs development, and you should not mix that up with what you send me. Linus
On Thu, Jul 18, 2024 at 03:06:07PM GMT, Linus Torvalds wrote: > On Thu, 18 Jul 2024 at 14:21, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > > > But my master branch (previously the same as for-next) will now be > > for-next merged with the latest tag from your tree, and I may do > > similarly for bcachefs-for-upstream if it's needed. > > No. No back-merges. We actually have documentation about this, so I > won't repeat the hundreds of emails I've sent out: Sorry, I must not have been clear. My master branch is a) not where I do development, and b) not not a branch that I will be sending to you - that's simply the primary branch I publish for people testing the latest bcachefs code. So: master will now be just updated by a hook on the server whenever I update for-next. > There are valid reasons to rebase, but they are balanced against some > VERY valid reasons not to do it, so if you have to rebase, make sure > those reasons really outweigh the reasons not to. > > And don't do it just before a pull request - and if there is some > catastrophic event that caused a recent rebase, let me know in the > pull request. Yes, this will help with that; last cycle I had to rebase at rc4 because of something my testers were hitting, but it wasn't affecting my development so with the new setup my development branches will be able to stay based on rc1. > > As a bonus, this means the testing automation will now be automatically > > testing my branch + your latest > > No. That's what linux-next is about - it does integration testing. Oh? Does it now? I've gotten essentially zero in the way of test feedback from for-next (except from Stephen Rothwell directly, the odd build warning or merge issue, but 0day mostly catches the build stuff before it hits next). And I don't think the rest of the filesystem community is any different, because a major subject at LSF this year was in fact the need for someone to start a fs-next branch for integration testing. > Your development branch IS NOT FOR TESTING OTHER RANDOM THINGS. > > You are actively making things worse if you do: you should worry about > YOUR code, not about all the random other things going on. > > Now, if you want to do _additional_ testing along the lines of "what > happens with my branch and Linus' latest" then that is ok - but that > should be something you see as completely separate from testing your > own work. Yes, that's exactly what I was describing. > > IOW, you can certainly wear "multiple hats" - your bcachefs developer > hat that worries about your bcachefs branch, and if you want to *also* > do some integration testing, go right ahead and have another > "integration testing branch" that you then test separately. > > But I don't want your integration path. When I get a bcachefs pull, I > want the *bcachefs* side to be solid and tested. Not something else. Ditto > So by all means keep multiple branches for different reasons. If you > think you have users that need to test some integration branch (which > honestly sounds like a horrible idea, but you do you), No, the integration branch isn't for testing other code, it's because they don't want to be running rc1 if rc4 or rc7 is out. That's literally all it is.
On Thu, 18 Jul 2024 at 15:24, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > Sorry, I must not have been clear. My master branch is a) not where I do > development, and b) not not a branch that I will be sending to you - > that's simply the primary branch I publish for people testing the latest > bcachefs code. > > So: master will now be just updated by a hook on the server whenever I > update for-next. Oh, ok, then that's fine. > > Now, if you want to do _additional_ testing along the lines of "what > > happens with my branch and Linus' latest" then that is ok - but that > > should be something you see as completely separate from testing your > > own work. > > Yes, that's exactly what I was describing. Good, sounds like we're on the same page. Linus
The pull request you sent on Sun, 14 Jul 2024 21:26:30 -0400:
> https://evilpiepirate.org/git/bcachefs.git tags/bcachefs-2024-07-14
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2e118ba36d56acf78084518dfb7cb53b1d417da0
Thank you!
On Thu, Jul 18, 2024 at 06:24:08PM -0400, Kent Overstreet wrote: > > I've gotten essentially zero in the way of test feedback from > for-next (except from Stephen Rothwell directly, the odd build > warning or merge issue, but 0day mostly catches the build stuff > before it hits next). I am currently running regular testing on the new linux-next's fs-next branch. Things which are still blocking me from announcing it are: *) Negotiating with Konstantin about the new lists.linux.dev mailing list. *) A few minor bug fixes / robustification improves in the "gce-xfstests watch" --- for example, right now if git fetch fails due to load throttling / anti-DOS protections on git.kernel.org trip the git watcher dies. Obviously, I need to teach it to do exponential backoff retries, because I'm not going to leave my kernel.org credentials on a VM running in the cloud to bypass the kernel.org DOS protections. :-) As far as bcachefs is concerned, my xfstests-bld infrastructure isn't set up to build rust userspace, and Debian has a very ancient bcachefs packages --- the latest version in Debian stable and unstable dates from November 2022. So I haven't enabled bcachefs support in gce-xfstests and kvm-xfstests yet. Patches gratefully accepted. :-) In any case, I'm hoping to have some publically accessible regular test results of fs-next. I've just been crazy busy lately.... - Ted
On Fri, Jul 19, 2024 at 10:30:01AM GMT, Theodore Ts'o wrote: > On Thu, Jul 18, 2024 at 06:24:08PM -0400, Kent Overstreet wrote: > > > > I've gotten essentially zero in the way of test feedback from > > for-next (except from Stephen Rothwell directly, the odd build > > warning or merge issue, but 0day mostly catches the build stuff > > before it hits next). > > I am currently running regular testing on the new linux-next's fs-next > branch. Things which are still blocking me from announcing it are: > > *) Negotiating with Konstantin about the new lists.linux.dev mailing > list. > > *) A few minor bug fixes / robustification improves in the > "gce-xfstests watch" --- for example, right now if git fetch fails > due to load throttling / anti-DOS protections on git.kernel.org > trip the git watcher dies. Obviously, I need to teach it to do > exponential backoff retries, because I'm not going to leave my > kernel.org credentials on a VM running in the cloud to bypass the > kernel.org DOS protections. :-) > > As far as bcachefs is concerned, my xfstests-bld infrastructure isn't > set up to build rust userspace, and Debian has a very ancient bcachefs > packages --- the latest version in Debian stable and unstable dates > from November 2022. So I haven't enabled bcachefs support in > gce-xfstests and kvm-xfstests yet. Patches gratefully accepted. :-) I can apt install 1.9.1? But I just discovered, because I had to switch my own testing to the debian package, that the mount helper wasn't being run previously (doesn't check /usr/local/sbin); and mounts with the mount helper are failing with -EBUSY. Presumably there's a race, since before calling the mount syscall, our mount helper has to open and close the block device (we have to read the superblock to check if it's encrypted, we may need to ask for a passphrase). The delayed_fput stuff that was causing issues with io_uring - I suspect something similar. But the plot thickens - all the failing tests are ones that use device mapper...
On Sat, Jul 20, 2024 at 11:48:09AM -0400, Kent Overstreet wrote: > > As far as bcachefs is concerned, my xfstests-bld infrastructure isn't > > set up to build rust userspace, and Debian has a very ancient bcachefs > > packages --- the latest version in Debian stable and unstable dates > > from November 2022. So I haven't enabled bcachefs support in > > gce-xfstests and kvm-xfstests yet. Patches gratefully accepted. :-) > > I can apt install 1.9.1? Ah, I see. bcachefs-tools is currently in Debian unstable, but not in Debian testing[1]; it's currently hung up due to the auto-libsodium transition[2]. Once that clears I can look at backporting it to debian-backports (since my test appliance runs on Debian stable, for better repeatable test appliance creation. :-) [1] https://tracker.debian.org/pkg/bcachefs-tools [2] https://release.debian.org/transitions/html/auto-libsodium.html - Ted