| Message ID | 1272723382-19470-2-git-send-email-orenl@cs.columbia.edu (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
NO WAY, there is no way in the world you should post 100 patches at a time to any mailing list, especially those at vger.kernel.org that have thousands upon thousands of subscribers. Post only small, well contained, sets of patches at a time. At most 10 or so in one go. Do you realize how much mail traffic you generate by posting so many patches at one time, and how unlikely it is for anyone to actually sift through and review your patches after you've spammed them by posting so many at one time? A second infraction and I will have no choice but to block you at the SMTP level at vger.kernel.org so please do not do it again. Thanks.
On Sat, May 01, 2010 at 03:10:22PM -0700, David Miller wrote: > >NO WAY, there is no way in the world you should post 100 patches >at a time to any mailing list, especially those at vger.kernel.org >that have thousands upon thousands of subscribers. > >Post only small, well contained, sets of patches at a time. At most >10 or so in one go. > >Do you realize how much mail traffic you generate by posting so many >patches at one time, and how unlikely it is for anyone to actually >sift through and review your patches after you've spammed them by >posting so many at one time? > >A second infraction and I will have no choice but to block you at the >SMTP level at vger.kernel.org so please do not do it again. So I really agree with everything you said here, but I do wonder why you haven't sent a similar rant about the often 100+ patchsets for the -stable series. We are supposed to review those and follow up on them to be sure they're suitable for a stable release. Or the 100+ emails about regressions from version to version. Etc, etc. I'm not saying you're wrong, but it does seem a bit odd that you choose to reply to this one, and not the other umpteen cases I often see. Maybe it isn't about the size or volume of the emails, and more about the fact that it's 100 patches to implement _one_ thing? If so, then I don't really think it's about list traffic at all... josh
On Sat, May 01, 2010 at 03:10:22PM -0700, David Miller wrote: > NO WAY, there is no way in the world you should post 100 patches > at a time to any mailing list, especially those at vger.kernel.org > that have thousands upon thousands of subscribers. I am sorry we concluded that sending these 100 patches at once was a good idea. I will try, again, to find ways to divide the set up into more manageable pieces. Regardless of how that goes the whole set will not be submitted to LKML/vger all at once in the future. If anyone would like to offer more specific constructive suggestions on subdividing the patches I'd be happy to try them. That said, for anyone who's curious, we faced a few dilemmas which pointed us down the wrong path here. http://lkml.org/lkml/2010/3/1/422 Specifically the last part is rather hard to misinterpret: "I'd suggest waiting until very shortly after 2.6.34-rc1 then please send all the patches onto the list and let's get to work." (ok, it's not shortly after 2.6.34-rc1 -- we were asked to reorganize the code and we did...) But even if one decides to ignore the common sense interpretation of Andrew's reply there was more: Standard procedure is to post to LKML when pushing patches upstream. We were asked to create a useful implementation of checkpoint/restart yet when we tried to submit a digestable piece we were told that submitting it by itself was pointless (eclone). The rest of the code was even more checkpoint/restart-specific so the same logic seemed to apply. We have public git trees and used the containers@ mailing list to post patches for review but rarely received outside feedback on patches there. Not even requests to divide the set. So clearly we needed to post to relevant external lists and reviewers. We tried that earlier and received complaints that lists hadn't been Cc'd on some of the patches (e.g. fsdevel). So clearly we needed to expand the Cc list for v21. We looked at dividing the set but it always came down to trimming functionality -- this conflicted with the "useful implementation" we were asked for. In summary: We've been given a fair number of conflicting instructions and we failed to find the right balance in following them. > Post only small, well contained, sets of patches at a time. At most > 10 or so in one go. We've tried to keep the individual patches small and reviewable. That has the opposite effect on patch count unfortunately. > > Do you realize how much mail traffic you generate by posting so many > patches at one time, and how unlikely it is for anyone to actually > sift through and review your patches after you've spammed them by > posting so many at one time? > > A second infraction and I will have no choice but to block you at the > SMTP level at vger.kernel.org so please do not do it again. We will not post nearly this many at once again. I'm thinking we'll just provide URLs to git trees or quilt series if subdividing is impossible and/or anyone needs wider context than the 10 or so we post at a time. Sorry again, -Matt Helsley
David Miller wrote: > NO WAY, there is no way in the world you should post 100 patches > at a time to any mailing list, especially those at vger.kernel.org > that have thousands upon thousands of subscribers. > > Post only small, well contained, sets of patches at a time. At most > 10 or so in one go. > > Do you realize how much mail traffic you generate by posting so many > patches at one time, and how unlikely it is for anyone to actually > sift through and review your patches after you've spammed them by > posting so many at one time? > > A second infraction and I will have no choice but to block you at the > SMTP level at vger.kernel.org so please do not do it again. Some people, you give them gold and they just complain it's heavy.
On Sat, 2010-05-01 at 15:10 -0700, David Miller wrote: > NO WAY, there is no way in the world you should post 100 patches > at a time to any mailing list, especially those at vger.kernel.org > that have thousands upon thousands of subscribers. > > Post only small, well contained, sets of patches at a time. At most > 10 or so in one go. Hi Dave, I really do apologize if these caused undue traffic on vger. It certainly wasn't our intention to cause any problems. We've had a fear all along that we'll just go back into our little containers@lists.linux-foundation.org, and go astray of what the community wants done with these patches. It's also important to remember that these really do affect the entire kernel. Unfortunately, it's not really a single feature or something that fits well on any other mailing list. It has implications _everywhere_. I think Andrew Morton also asked that these continue coming to LKML, although his request probably came at a time when the set was a wee bit smaller. > Do you realize how much mail traffic you generate by posting so many > patches at one time, and how unlikely it is for anyone to actually > sift through and review your patches after you've spammed them by > posting so many at one time? I honestly don't expect people to be reading the whole thing at once. But, I do hope that people can take a look at their individual bits that are touched. > A second infraction and I will have no choice but to block you at the > SMTP level at vger.kernel.org so please do not do it again. I know these patches are certainly not at the level of importance as, say the pata or -stable updates, but they're certainly not of unprecedented scale. I've seen a number of patchbombs of this size recently. I hope Andrew pulls this set into -mm so this doesn't even come up again. But, if it does, can you make some suggestions on how to be more kind to vger in the process? -- Dave
From: Dave Hansen <dave@linux.vnet.ibm.com> Date: Mon, 03 May 2010 14:02:31 -0700 > It has implications _everywhere_. That does not remove the responsibility to break things up into managable pieces, not does it make such a task impossible or even hard to do. You post sets of 10 to 15 at a time, once those are agreed to and to everyone's general liking, you toss them into a GIT tree and you say "here's the next 10 to 15 and they are relative to the changes in GIT tree X which have already been fully reviewed" And so on and so forth. And this is the only logical thing to do, because if someone wants a change in patch 7, it can effect patch 23 so it's pointless to post for review a patch that's going to end up changing anyways. That's a waste of reviewer resources. To be honest, I'm really tired of what tends to be people's knee jerk reaction to this situations, which is a lot of people doing nothing but defending themselves. Even if it did not violate documented policy (it did), it violates common sense. So, can people do something more constructive than trying to defend themselves on this? It's stupid and shouldn't have been done, and we should move on.
With a huge patch series like this, can you post a cover note at the front (usually patch 0) saying what the point of the whole series is? David
Hi David, I suppose you are looking for more details than those found in the current patch-0 (http://lkml.org/lkml/2010/5/1/140). We omitted them for brevity sake; here is a link to patch-0 of a previous post of the patchset: http://lkml.org/lkml/2009/9/23/423 Thanks, Oren. David Howells wrote: > With a huge patch series like this, can you post a cover note at the front > (usually patch 0) saying what the point of the whole series is? > > David >
diff --git a/kernel/pid.c b/kernel/pid.c index aebb30d..52a371a 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -122,6 +122,30 @@ static void free_pidmap(struct upid *upid) atomic_inc(&map->nr_free); } +static int alloc_pidmap_page(struct pidmap *map) +{ + void *page; + + if (likely(map->page)) + return 0; + + page = kzalloc(PAGE_SIZE, GFP_KERNEL); + /* + * Free the page if someone raced with us installing it: + */ + spin_lock_irq(&pidmap_lock); + if (!map->page) { + map->page = page; + page = NULL; + } + spin_unlock_irq(&pidmap_lock); + kfree(page); + if (unlikely(!map->page)) + return -1; + + return 0; +} + static int alloc_pidmap(struct pid_namespace *pid_ns) { int i, offset, max_scan, pid, last = pid_ns->last_pid; @@ -134,22 +158,9 @@ static int alloc_pidmap(struct pid_namespace *pid_ns) map = &pid_ns->pidmap[pid/BITS_PER_PAGE]; max_scan = (pid_max + BITS_PER_PAGE - 1)/BITS_PER_PAGE - !offset; for (i = 0; i <= max_scan; ++i) { - if (unlikely(!map->page)) { - void *page = kzalloc(PAGE_SIZE, GFP_KERNEL); - /* - * Free the page if someone raced with us - * installing it: - */ - spin_lock_irq(&pidmap_lock); - if (!map->page) { - map->page = page; - page = NULL; - } - spin_unlock_irq(&pidmap_lock); - kfree(page); - if (unlikely(!map->page)) + if (unlikely(!map->page)) + if (alloc_pidmap_page(map) < 0) break; - } if (likely(atomic_read(&map->nr_free))) { do { if (!test_and_set_bit(offset, map->page)) {