Messages in this thread | | | Date | Thu, 20 Sep 2012 17:18:14 -0400 | From | Konrad Rzeszutek Wilk <> | Subject | Re: 3.6rc6 slab corruption. |
| |
On Wed, Sep 19, 2012 at 07:46:57PM -0700, David Rientjes wrote: > On Thu, 20 Sep 2012, Raghavendra K T wrote: > > > Only problem, I find is histogram data expands dynamically (because it > > changes). I think having static allocation of 352 bytes as suggested > > Linus is a good idea. > > > > Certainly, but it's a different topic and would be a subsequent patch to > either my patch or Konrad's patch. Before that's done, I think we should > fix the race condition that currently exists either by: > > - merging my patch (which I can sign-off and write a changelog for if > Konrad agrees), or
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > - merging Konrad's patch and introducing a mutex so that it's possible to > do many reads to collect statistics after opening the file a single > time with a single fd. > > Since these files are incapable of doing lseek, it would seem that my > patch would be best and you'd simply want to close() + open() to read new > data, which also guarantees that all readers get the same information.
Yup. > The only reason I hesitate on that and will defer to Konrad's opinion is > because the way the code is currently written looks like it was intended > to copy the data are read() rather than open(); in other words, it almost > seems as if they were made to be non-seekable after the u32_array_read() > implementation was complete and it was at one time possible to do an > lseek(SEEK_SET).
The "users" (looks at himself and Raghavendra) can deal with the open/close, open/close cycle. The only thing that I would add extra is to add the explanation you provided in the comment of the file in case somebody expects something else. > > After that's fixed, and to address your concern, we can simply do the > allocation of file->private_data for the maximum size possible when the > file is created as a follow-up patch.
| |