[bug?] dumparchive behaviour
Rohit Sarkar
rohitsarkar5398 at gmail.com
Sun Jul 12 23:30:56 AEST 2020
Hey,
On Sat, Jul 11, 2020 at 08:04:59AM +0530, Rohit Sarkar wrote:
> Hey,
> While using dumparchive on a Patchwork instance with some patches from
> the linux-mm mailing list I noticed something weird in the mbox archive
> generated.
>
> Some patches dont contain a patchwork id or message id in the dump, but
> I noticed what seemed like a duplicate patch in the same dump which does
> contain both.
>
> Attaching an extract from the archive for reference. In the extract the
> first patch does contain both the ids, but the second doesnt. Is the
> second one required in the dump? If so shouldnt it have the respective
> ids.
>
> Thanks,
> Rohit
>
>
> From patchwork Mon Jan 4 05:52:36 2010
> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> X-Patchwork-Submitter: KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com>
> X-Patchwork-Id: 10690
> Return-Path: <owner-linux-mm at kvack.org>
> Received: from mail172.messagelabs.com (mail172.messagelabs.com
> [216.82.254.3])
> by kanga.kvack.org (Postfix) with SMTP id 3F61B600068
> for <linux-mm at kvack.org>; Mon, 4 Jan 2010 00:52:41 -0500 (EST)
> Received: from m6.gw.fujitsu.co.jp ([10.0.50.76])
> by fgwmail6.fujitsu.co.jp (Fujitsu Gateway) with ESMTP id o045qbnQ014218
> for <linux-mm at kvack.org> (envelope-from kosaki.motohiro at jp.fujitsu.com);
> Mon, 4 Jan 2010 14:52:38 +0900
> Received: from smail (m6 [127.0.0.1])
> by outgoing.m6.gw.fujitsu.co.jp (Postfix) with ESMTP id 9D5B045DE50
> for <linux-mm at kvack.org>; Mon, 4 Jan 2010 14:52:37 +0900 (JST)
> Received: from s6.gw.fujitsu.co.jp (s6.gw.fujitsu.co.jp [10.0.50.96])
> by m6.gw.fujitsu.co.jp (Postfix) with ESMTP id 6FB6145DE4C
> for <linux-mm at kvack.org>; Mon, 4 Jan 2010 14:52:37 +0900 (JST)
> Received: from s6.gw.fujitsu.co.jp (localhost.localdomain [127.0.0.1])
> by s6.gw.fujitsu.co.jp (Postfix) with ESMTP id 4C2801DB803F
> for <linux-mm at kvack.org>; Mon, 4 Jan 2010 14:52:37 +0900 (JST)
> Received: from m105.s.css.fujitsu.com (m105.s.css.fujitsu.com [10.249.87.105])
> by s6.gw.fujitsu.co.jp (Postfix) with ESMTP id F2E261DB8037
> for <linux-mm at kvack.org>; Mon, 4 Jan 2010 14:52:36 +0900 (JST)
> From: KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com>
> Subject: [PATCH] page allocator: fix update NR_FREE_PAGES only as necessary
> In-Reply-To: <20100104122138.f54b7659.minchan.kim at barrios-desktop>
> References: <1262571730-2778-1-git-send-email-shijie8 at gmail.com>
> <20100104122138.f54b7659.minchan.kim at barrios-desktop>
> Message-Id: <20100104144332.96A2.A69D9226 at jp.fujitsu.com>
> MIME-Version: 1.0
> Date: Mon, 4 Jan 2010 14:52:36 +0900 (JST)
> Sender: owner-linux-mm at kvack.org
> To: Minchan Kim <minchan.kim at gmail.com>
> Cc: kosaki.motohiro at jp.fujitsu.com, Huang Shijie <shijie8 at gmail.com>,
> akpm at linux-foundation.org, mel at csn.ul.ie, linux-mm at kvack.org
> List-ID: <linux-mm.kvack.org>
>
> > Hi, Huang.
> >
> > On Mon, 4 Jan 2010 10:22:10 +0800
> > Huang Shijie <shijie8 at gmail.com> wrote:
> >
> > > When the `page' returned by __rmqueue() is NULL, the origin code
> > > still adds -(1 << order) to zone's NR_FREE_PAGES item.
> > >
> > > The patch fixes it.
> > >
> > > Signed-off-by: Huang Shijie <shijie8 at gmail.com>
> > > ---
> > > mm/page_alloc.c | 10 +++++++---
> > > 1 files changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 4e9f5cc..620921d 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -1222,10 +1222,14 @@ again:
> > > }
> > > spin_lock_irqsave(&zone->lock, flags);
> > > page = __rmqueue(zone, order, migratetype);
> > > - __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
> > > - spin_unlock(&zone->lock);
> > > - if (!page)
> > > + if (likely(page)) {
> > > + __mod_zone_page_state(zone, NR_FREE_PAGES,
> > > + -(1 << order));
> > > + spin_unlock(&zone->lock);
> > > + } else {
> > > + spin_unlock(&zone->lock);
> > > goto failed;
> > > + }
> > > }
> > >
> > > __count_zone_vm_events(PGALLOC, zone, 1 << order);
> >
> > I think it's not desirable to add new branch in hot-path even though
> > we could avoid that.
> >
> > How about this?
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 4e4b5b3..87976ad 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1244,6 +1244,9 @@ again:
> > return page;
> >
> > failed:
> > + spin_lock(&zone->lock);
> > + __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
> > + spin_unlock(&zone->lock);
> > local_irq_restore(flags);
> > put_cpu();
> > return NULL;
>
> Why can't we write following? __mod_zone_page_state() only require irq
> disabling, it doesn't need spin lock. I think.
>
>
>
> From 72011ff2b0bba6544ae35c6ee52715c8c824a34b Mon Sep 17 00:00:00 2001
> From: KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com>
> Date: Mon, 4 Jan 2010 14:38:20 +0900
> Subject: [PATCH] page allocator: fix update NR_FREE_PAGES only as necessary
>
> commit f2260e6b (page allocator: update NR_FREE_PAGES only as necessary)
> made one minor regression.
> if __rmqueue() was failed, NR_FREE_PAGES stat go wrong. this patch fixes
> it.
>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com>
> Cc: Mel Gorman <mel at csn.ul.ie>
> Cc: Huang Shijie <shijie8 at gmail.com>
> Cc: Minchan Kim <minchan.kim at gmail.com>
> ---
> mm/page_alloc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 11ae66e..ecf75a1 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1227,10 +1227,10 @@ again:
> }
> spin_lock_irqsave(&zone->lock, flags);
> page = __rmqueue(zone, order, migratetype);
> - __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
> spin_unlock(&zone->lock);
> if (!page)
> goto failed;
> + __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
> }
>
> __count_zone_vm_events(PGALLOC, zone, 1 << order);
So I did some investigation on this. This [1] is the patch in question
on lore.kernel.org. It weirdly contains a patch within a patch.
Patchwork treats it as one single patch, I am not sure if that's the right
approach here.
Thanks,
Rohit
[1]: https://lore.kernel.org/linux-mm/20100104144332.96A2.A69D9226@jp.fujitsu.com/
More information about the Patchwork
mailing list