* linux-3.12 userspace @ 2013-10-25 13:19 Rich Johnston 2013-10-25 15:02 ` Eric Sandeen 2013-10-28 22:40 ` linux-3.12 userspace Take 2 Rich Johnston 0 siblings, 2 replies; 7+ messages in thread From: Rich Johnston @ 2013-10-25 13:19 UTC (permalink / raw) To: xfs-oss Hey Folks, Dave Chinner has a 32 part userspace patchset that needs to be reviewed and will be committed to coincide with the linux-3.12 kernel release. Are there other userspace patches that support 3.12 kernel features that need to be worked? All other patches on the list will be held back until the linux-3.13 merge window has opened. Thanks --Rich _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: linux-3.12 userspace 2013-10-25 13:19 linux-3.12 userspace Rich Johnston @ 2013-10-25 15:02 ` Eric Sandeen 2013-10-25 17:55 ` Rich Johnston 2013-10-28 22:40 ` linux-3.12 userspace Take 2 Rich Johnston 1 sibling, 1 reply; 7+ messages in thread From: Eric Sandeen @ 2013-10-25 15:02 UTC (permalink / raw) To: Rich Johnston, xfs-oss On 10/25/13 8:19 AM, Rich Johnston wrote: > Hey Folks, > > Dave Chinner has a 32 part userspace patchset that needs to be reviewed > and will be committed to coincide with the linux-3.12 kernel release. > > Are there other userspace patches that support 3.12 kernel features that > need to be worked? > > All other patches on the list will be held back until the linux-3.13 > merge window has opened. Are you talking about holding back kernelspace or userspace patches? We'd want to get kernelspace merged in the xfs git tree well before the merge window, and I don't think the kernel merge window needs to affect userspace merges. Can you talk in more specifics (which series/patches, for what codebase) you're proposing? In general I think we simply have a review bottleneck, but once patchsets are reviewed, in general, they should just get merged, especially in userspace, IMHO. But maybe I just need more details. :) -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: linux-3.12 userspace 2013-10-25 15:02 ` Eric Sandeen @ 2013-10-25 17:55 ` Rich Johnston 2013-10-28 2:23 ` Dave Chinner 0 siblings, 1 reply; 7+ messages in thread From: Rich Johnston @ 2013-10-25 17:55 UTC (permalink / raw) To: Eric Sandeen, xfs-oss On 10/25/2013 10:02 AM, Eric Sandeen wrote: > On 10/25/13 8:19 AM, Rich Johnston wrote: >> Hey Folks, >> >> Dave Chinner has a 32 part userspace patchset that needs to be reviewed >> and will be committed to coincide with the linux-3.12 kernel release. I was referring to userspace ( hence the subject ;) ) matching kernel. >> >> Are there other userspace patches that support 3.12 kernel features that >> need to be worked? >> >> All other patches on the list will be held back until the linux-3.13 >> merge window has opened. > > Are you talking about holding back kernelspace or userspace patches? Only userspace. > > We'd want to get kernelspace merged in the xfs git tree well before > the merge window, Yup Ben is working on it. > and I don't think the kernel merge window needs to > affect userspace merges. Umm yes I need to wait until the kernel supports the feature before adding it to userspace. AFAIR the goal was to have userspace features match the kernelspace. > > Can you talk in more specifics (which series/patches, for what codebase) > you're proposing? Yes I was referring to "[PATCH 00/32] xfsprogs: V5 write support for xfs_db" http://oss.sgi.com/archives/xfs/2013-09/msg00805.html Which needs to be reviewed before I can pull it in. I was asking for any other userspace patches that need to be pulled in for the next userspace release so it matches kernel 3.12. > > In general I think we simply have a review bottleneck, but once patchsets > are reviewed, in general, they should just get merged, especially in userspace, > IMHO. > No I was chastised for pulling in reviewed userspace patches too early. Kernel code was not fully hashed out. > But maybe I just need more details. :) Am I still missing details? > > -Eric > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: linux-3.12 userspace 2013-10-25 17:55 ` Rich Johnston @ 2013-10-28 2:23 ` Dave Chinner 0 siblings, 0 replies; 7+ messages in thread From: Dave Chinner @ 2013-10-28 2:23 UTC (permalink / raw) To: Rich Johnston; +Cc: Eric Sandeen, xfs-oss On Fri, Oct 25, 2013 at 12:55:03PM -0500, Rich Johnston wrote: > On 10/25/2013 10:02 AM, Eric Sandeen wrote: > >On 10/25/13 8:19 AM, Rich Johnston wrote: > >>Hey Folks, > >> > >>Dave Chinner has a 32 part userspace patchset that needs to be > >>reviewed and will be committed to coincide with the linux-3.12 > >>kernel release. > > I was referring to userspace ( hence the subject ;) ) matching > kernel. Kernel and userspace don't release on the same schedule. > >We'd want to get kernelspace merged in the xfs git tree well > >before the merge window, > > Yup Ben is working on it. Everyone can review kernel patches, not just Ben. The more eyes looking at the code the faster the process will go. > >and I don't think the kernel merge window needs to affect > >userspace merges. > > Umm yes I need to wait until the kernel supports the feature > before adding it to userspace. AFAIR the goal was to have > userspace features match the kernelspace. The issue is not todo with release schedules, but whether patches are committed to the git trees or not. That is, for code shared between kernel and userspace, the process is effectively: kernel side: userspace side: propose patch propose equivalent patch review cycle: review cycle: address comments address comments update with userspace changes update with kernel side changes test test repost repost reviewed-by given commit to xfs-oss tree update match kernel commit repost commit to xfsprogs tree However, for pure userspace patches (like the write support for xfs_db patches), there is not co-ordination needed with the kernel patches. Those patches can be reviewed immediately, even if they have a dependency on the shared code patches. All that means is that they have to be committed *after* the above process for shared patches completes. > >Can you talk in more specifics (which series/patches, for what > >codebase) you're proposing? > > Yes I was referring to "[PATCH 00/32] xfsprogs: V5 write support > for xfs_db" http://oss.sgi.com/archives/xfs/2013-09/msg00805.html > Which needs to be reviewed before I can pull it in. what that userspace series looks like from a process point of view is this: First N patches: - shared code already committed to kernel - review and commit required only Second M patches - shared code not yet committed to kernel - kernel and user review process as per above Last O patches - userspace only patches - review and commit required only - commit dependent on previous shared patches being ready for commit. IOWs, N has no dependency, M are dependent on kernel commits, and O are dependent only on N and M being committed to userspace. At no stage does the fact that committing M first requires kernel commits prevent review of N, M or O from occurring.... i.e. we can review userspace code without needing to immediately commit it! > I was asking for any other userspace patches that need to be > pulled in for the next userspace release so it matches kernel > 3.12. We don't do userspace releases to match kernel releases. They are independent and asynchronous. We try to have functional userspace changes committed to the dev tree to match kernel releases (e.g. mkfs/repair support for a new feature) but that doesn't mean that we need exact code parity between the kernel and userspace at the same time. > >In general I think we simply have a review bottleneck, but once > >patchsets are reviewed, in general, they should just get merged, > >especially in userspace, IMHO. > > No I was chastised for pulling in reviewed userspace patches too > early. Kernel code was not fully hashed out. Sure, but that has nothing to do with release schedules for kernel and userspace code. That's a process problem, and one that should not happen because we've explained the reason for the process being the way it is several times in the past. It was highlighted again most recently with the build breakage that Mark caused by porting a userspace patch back to the the kernel and then not compile testing it before it was committed... Ben, as the XFS Maintainer, it is your responsibility to ensure that engineers doing work on your behalf understand what they are doing and what processes they need to follow. Can you please get together with Rich and provide him with the knowledge and oversight he needs to understand how to manage multiple developers and large patch series so he can avoid making further mistakes? Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* linux-3.12 userspace Take 2 2013-10-25 13:19 linux-3.12 userspace Rich Johnston 2013-10-25 15:02 ` Eric Sandeen @ 2013-10-28 22:40 ` Rich Johnston 2013-10-29 21:30 ` Eric Sandeen 2013-10-29 22:05 ` Dave Chinner 1 sibling, 2 replies; 7+ messages in thread From: Rich Johnston @ 2013-10-28 22:40 UTC (permalink / raw) To: xfs-oss Hey Folks, Sorry for any confusion, let me try again. In preparation for the new userspace release, are there any outstanding userspace patches that should be marked as critical and hold up the new userspace release? Code shared by userspace and kernelspace are committed by different maintainers, I propose we discuss how to make it clear which patch series are tied together. This would aid reviewers and testers also. One thought is to state in the kernel [PATCH 0/XX] email body something like: This kernel series shares the same headers as the userspace series "NAME OF USERSPACE SERIES" and a similar email for the userspace series. The second commit should contain the first series commit id to tie them together. Userspace patch series will be committed when the entire patch series has been reviewed. Partial series commits will happen only with the authors approval (confirmation posted to the list). Thanks --Rich _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: linux-3.12 userspace Take 2 2013-10-28 22:40 ` linux-3.12 userspace Take 2 Rich Johnston @ 2013-10-29 21:30 ` Eric Sandeen 2013-10-29 22:05 ` Dave Chinner 1 sibling, 0 replies; 7+ messages in thread From: Eric Sandeen @ 2013-10-29 21:30 UTC (permalink / raw) To: Rich Johnston, xfs-oss On 10/28/13 5:40 PM, Rich Johnston wrote: > Hey Folks, > > Sorry for any confusion, let me try again. > > In preparation for the new userspace release, are there any outstanding > userspace patches that should be marked as critical and hold up the new > userspace release? Dave should definitely chime in too, but certainly at least the xfs_db patchset - and talking w/ Dave, there's still much repair work to be done. I'd personally like to see my xfs_fsr fix make it, but that's pretty small potatoes. However, I'm not sure we need to be super "release" focused quite yet; more emphasis on reviewing & merging what's on the list will keep it all moving along, and we can worry about releases down the road. Once the xfs_db stuff goes in, maybe that would warrant another prerelease, though. > Code shared by userspace and kernelspace are committed by different > maintainers, I propose we discuss how to make it clear which patch > series are tied together. This would aid reviewers and testers also. > > One thought is to state in the kernel [PATCH 0/XX] email body > something like: > This kernel series shares the same headers as the > userspace series "NAME OF USERSPACE SERIES" > > and a similar email for the userspace series. > > The second commit should contain the first series commit id to > tie them together. Having two maintainers will make it more tricky, and you guys will have to communicate well in order for things to go smoothly. To be orderly about it, I think we must be in a "kernel comes first" mindset, i.e. changes are made there first, and then userspace, whether we're talking about shared code, or new interfaces. That'll save us from "free()" type errors, among other things. ;) (There will be some independent changes on both sides, but determining that independence should be part of review, i.e. "hey you need to change this in kernelspace first!") So yes, if you have a userspace patchset that has kernelspace counterparts, wait for them to be committed by Ben. And make sure they still match when they go into xfsprogs; if not, that should be noted, and the patch series reposted. As long as kernelspace is being committed in a timely manner post-review, then userspace shouldn't lag that by much at all. I can see how it might be helpful if a 00/XX cover letter makes it clear that i.e. "patches 1 through 4 are to match kernel changes which have already been sent in series $BLAH" or "... which are already committed." If you want to adjust commit logs to note the kernel commit, you could, although sounds like a bit more work on the maintainer end, and maybe of limited value. In the long run, I think Dave was talking about not a copy, but actual shared code for libxfs, which would simplify all this. But we're not there yet ... > Userspace patch series will be committed when the entire patch series > has been reviewed. Partial series commits will happen only with the > authors approval (confirmation posted to the list). Yeah, I think that's the best plan. If it seems that a large series is accomplishing several discrete changes, that could be noted in the cover letter as well, giving "permission" up front to do a partial merge when independent bits get reviewed, so the whole series doesn't need to wait if it doesn't have to. -Eric > Thanks > --Rich > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: linux-3.12 userspace Take 2 2013-10-28 22:40 ` linux-3.12 userspace Take 2 Rich Johnston 2013-10-29 21:30 ` Eric Sandeen @ 2013-10-29 22:05 ` Dave Chinner 1 sibling, 0 replies; 7+ messages in thread From: Dave Chinner @ 2013-10-29 22:05 UTC (permalink / raw) To: Rich Johnston; +Cc: xfs-oss On Mon, Oct 28, 2013 at 05:40:15PM -0500, Rich Johnston wrote: > Hey Folks, > > Sorry for any confusion, let me try again. > > In preparation for the new userspace release, are there any outstanding > userspace patches that should be marked as critical and hold up the new > userspace release? If you are talking about 3.2.0, then we are not yet even ready for a beta release as xfs_repair is not yet fully complete. There's still a siginificant amount of work needed to complete that. e.g: - dirent ftype check/repair/rebuild - repair throws CORRUPTION_ERROR output from verifiers on broken filesystems instead of detecting and handling it - repair doesn't check for CRC errors and consider them an error that needs fixing at all - log zeroing invalidates all the metadata LSNs in the filesystem, so we need to track the maximum LSN in filesystem metadata and write a dummy record into the log with that in it if we've zeroed the log There's a few other bits of new metadata that aren't fully validated/repaired, either, so there's still a bunch of infrastructure and repair work to be done before we can release a 3.2.0 package. I'd say there's at least 50-100 patches in the above work still to be done.... > Code shared by userspace and kernelspace are committed by different > maintainers, I propose we discuss how to make it clear which patch > series are tied together. Yes, I do that already. > This would aid reviewers and testers also. > > One thought is to state in the kernel [PATCH 0/XX] email body > something like: > This kernel series shares the same headers as the > userspace series "NAME OF USERSPACE SERIES" > > and a similar email for the userspace series. e.g. from the last xfs_db write series I posted: "The first part of the patch series fixes a couple of minor bugs, followed by syncing up with the kernel code to match the series I just posted for 3.13. This is necessary for CRC support in xfs_db." http://oss.sgi.com/archives/xfs/2013-09/msg00805.html > The second commit should contain the first series commit id to > tie them together. Extracting commit ids from a different repository and then matching and adding them to patch headers individually is a painful, painful process. If the reviewers can't run a "git log" command themselves to check that the patch is in the kernel code, then I'd say they simply aren't qualified to review the code in the patch.... > Userspace patch series will be committed when the entire patch > series has been reviewed. Partial series commits will happen only > with the authors approval (confirmation posted to the list). Use your discretion. If a patch series is made up of "sync to kernel code" and "add new functionality", then the first "sync to kernel code" part can be committed before the "add new functionality" part has been fully reviewed. If you are not sure, ask on the list. It doesn't matter if the author doesn't respond - someone else can say "yes, makes sense to commit that portion"... However, this does not address the root cause of the problems that have lead to this discussion. It's the same problem we've had for the past year - patches sitting around for weeks or months before anyone other than me looks at them. That's the real problem here - it's now 8 weeks since code that is a blocker for a 3.2.0 release was first posted, and not a single comment has been made about it. All this talk about kernel/userspace syncing is just a side show - it's never been a major issue before and it's not a major issue now. What is a major issue is that code is going unreviewed for weeks/months on end and we've been having this problem for at least a year now. So let's try to address the real problem that needs solving here - getting patches reviewed and committed *promptly*. We don't need more process overhead to accomplish that - we need people to actually put time into reviewing code quickly and we need maintainers that commit reviewed patch series just as quickly. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-10-29 22:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-25 13:19 linux-3.12 userspace Rich Johnston 2013-10-25 15:02 ` Eric Sandeen 2013-10-25 17:55 ` Rich Johnston 2013-10-28 2:23 ` Dave Chinner 2013-10-28 22:40 ` linux-3.12 userspace Take 2 Rich Johnston 2013-10-29 21:30 ` Eric Sandeen 2013-10-29 22:05 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox