RE: Hugetlb demanding paging for -mm tree

Chen, Kenneth W
2004-08-06T21:06:03Z

Andi Kleen wrote on Thursday, August 05, 2004 7:05 AM
> On Thu, Aug 05, 2004 at 06:42:15AM -0700, Chen, Kenneth W wrote:
> > +int hugetlb_acct_memory(long delta)
> > +{
> > + atomic_add(delta, &hugetlbzone_resv);
> > + if (delta > 0 && atomic_read(&hugetlbzone_resv) >
> > + VMACCTPG(hugetlb_total_pages())) {
> > + atomic_add(-delta, &hugetlbzone_resv);
> > + return -ENOMEM;
> > + }
> > + return 0;
>
> Wouldn't this be safer with a bit of locking?
> Even if the current code works lockless it would be more safer
> for long term mainteance.

Patch at the end of the mail. Is it better?

> > +}
> > +
> > +struct file_region {
> > + struct list_head link;
> > + int from;
> > + int to;
>
> Shouldn't these be long instead of int?

Yes, it should be long. Thank you.

>
> I remember writing very similar, but simpler code for NUMA API
> regions. The PAT patches also have similar code.
> It's also tricky to get right.
>
> Maybe it would be time to move variable length region list handling
> into a nice library in lib/, so that it can be used by other users.

Let me roll that up in the next couple of days along with the above struct
type change. For now I guess we can let it sit in hugetlbfs for a few days.

diff -Nurp linux-2.6.7/fs/hugetlbfs/inode.c linux-2.6.7.hugetlb/fs/hugetlbfs/inode.c
--- linux-2.6.7/fs/hugetlbfs/inode.c 2004-08-06 11:45:04.000000000 -0700
+++ linux-2.6.7.hugetlb/fs/hugetlbfs/inode.c 2004-08-06 13:46:11.000000000 -0700
@@ -36,17 +36,21 @@
#define VMACCT(x) ((x) >> (HPAGE_SHIFT))
#define VMACCTPG(x) ((x) >> (HPAGE_SHIFT - PAGE_SHIFT))

-atomic_t hugetlbzone_resv = ATOMIC_INIT(0);
+static long hugetlbzone_resv;
+static spinlock_t hugetlbfs_lock = SPIN_LOCK_UNLOCKED;

int hugetlb_acct_memory(long delta)
{
- atomic_add(delta, &hugetlbzone_resv);
- if (delta > 0 && atomic_read(&hugetlbzone_resv) >
- VMACCTPG(hugetlb_total_pages())) {
- atomic_add(-delta, &hugetlbzone_resv);
- return -ENOMEM;
- }
- return 0;
+ int ret = 0;
+
+ spin_lock(&hugetlbfs_lock);
+ if (delta > 0 && (hugetlbzone_resv + delta) >
+ VMACCTPG(hugetlb_total_pages()))
+ ret = -ENOMEM;
+ else
+ hugetlbzone_resv += delta;
+ spin_unlock(&hugetlbfs_lock);
+ return ret;
}

struct file_region {
@@ -225,8 +229,7 @@ static void hugetlb_acct_release(struct

int hugetlbfs_report_meminfo(char *buf)
{
- long htlb = atomic_read(&hugetlbzone_resv);
- return sprintf(buf, "HugePages_Reserved: %5lu\n", htlb);
+ return sprintf(buf, "HugePages_Reserved: %5lu\n", hugetlbzone_resv);
}

static struct super_operations hugetlbfs_ops;