Re: [PATCH] hugetlbfs read() support

Nishanth Aravamudan
2007-07-23T14:21:05Z

On 20.07.2007 [14:47:31 +1000], Nick Piggin wrote:
> Nishanth Aravamudan wrote:
> >On 19.07.2007 [09:58:50 -0700], Andrew Morton wrote:
> >
> >>On Thu, 19 Jul 2007 08:51:49 -0700 Badari Pulavarty <pbadari@us.ibm.com>
> >>wrote:
> >>
> >>
> >>>>>+ }
> >>>>>+
> >>>>>+ offset += ret;
> >>>>>+ retval += ret;
> >>>>>+ len -= ret;
> >>>>>+ index += offset >> HPAGE_SHIFT;
> >>>>>+ offset &= ~HPAGE_MASK;
> >>>>>+
> >>>>>+ page_cache_release(page);
> >>>>>+ if (ret == nr && len)
> >>>>>+ continue;
> >>>>>+ goto out;
> >>>>>+ }
> >>>>>+out:
> >>>>>+ return retval;
> >>>>>+}
> >>>>
> >>>>This code doesn't have all the ghastly tricks which we deploy to
> >>>>handle concurrent truncate.
> >>>
> >>>Do I need to ? Baaahh!! I don't want to deal with them.
> >>
> >>Nick, can you think of any serious consequences of a read/truncate
> >>race in there? I can't..
> >>
> >>
> >>>All I want is a simple read() to get my oprofile working. Please
> >>>advise.
> >>
> >>Did you consider changing oprofile userspace to read the executable
> >>with mmap?
> >
> >
> >It's not actually oprofile's code, though, it's libbfd (used by
> >oprofile). And it works fine (presumably) for other binaries.
>
> So... what's the problem with changing it? The fact that it is a
> library doesn't really make a difference except that you'll also help
> everyone else who links with it.

Well, I'm more concerned about testing that change libbfd is rather core
code and used in a number of places. Also, libbfd's current code 'just
works' for every other filesystem concerned, or I'd expect it would have
been changed to mmap() before. I'm also terrified of binutils code :)
I'm also not sure who I'm 'helping', exactly by changing it, beyond
users of libhugetlbfs and OProfile, who are equally helped by this
kernel patch (which, again, also has the added benefit of making
hugetlbfs appear to be more like a normal filesystem).

> It won't break backwards compatibility, and it will work on older
> kernels...

Fair enough. I'm looking into it, but I can't make any promises on
timelines.

Thanks,
Nish

--
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center