mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Nish Aravamudan" <nish.aravamudan@gmail.com>
To: "Eric Paris" <eparis@redhat.com>
Cc: wli@holomorphy.com, linux-kernel@vger.kernel.org, agl@us.ibm.com
Subject: Re: [PATCH] Hugetlb: stop race when reading proc meminfo
Date: Wed, 2 Jul 2008 15:51:35 -0700	[thread overview]
Message-ID: <29495f1d0807021551l1c934e68o3471105170ea0334@mail.gmail.com> (raw)
In-Reply-To: <1213899342.16752.39.camel@localhost.localdomain>

On 6/19/08, Eric Paris <eparis@redhat.com> wrote:
> minor nit that hugetlb_report_{,node_}meminfo() does not lock the
>  reading of nr_huge_pages, free_huge_pages, and friends so this is not an
>  atomic set of information put into the buffer.  If /proc/meminfo is read
>  while the number of hugetlb pages is in flux it is possible to get
>  incorrect output such as:
>
>  HugePages_Total:     7
>  HugePages_Free:      8
>  HugePages_Rsvd:      0
>  Hugepagesize:     4096 kB
>
>  (test available at https://bugzilla.redhat.com/attachment.cgi?id=309864)
>
>  With the patch we beat on a number of boxes for hours with the above
>  test and saw no inconsistencies in the meminfo output.
>
>  Signed-off-by: Eric Paris <eparis@redhat.com>

While I understand the spirit of this patch, I'm not sure it's really
necessary. We've never bothered locking the output in the proc file
before because the information is inherently racy. It only gives you a
view of what was...if any application tries to make a decision based
upon that information (unless it has global control of the system, or
is a testcase, arguably), then it is already racy -- that is, the
numbers you read at one point in time have no bearing on whether those
pages will actually be free/available, etc when you actually need
them.

That being said, I don't feel to strongly about it. Just seems like it
might be unnecessary for an interface we know not to be fully accurate
to begin with, but maybe consistency is worth a bit of extra locking.

That actually also leads me to wonder if maybe the locking is not
there currently to avoid letting readers of proc files from blocking
users of hugepages?

Thanks,
Nish

  reply	other threads:[~2008-07-02 22:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-19 18:15 Eric Paris
2008-07-02 22:51 ` Nish Aravamudan [this message]
2008-07-03 14:13   ` Adam Litke
2008-07-08 13:51     ` Eric Paris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=29495f1d0807021551l1c934e68o3471105170ea0334@mail.gmail.com \
    --to=nish.aravamudan@gmail.com \
    --cc=agl@us.ibm.com \
    --cc=eparis@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wli@holomorphy.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®