Re: [RFC] [PATCH 0/6] Provide cgroup isolation for buffered writes.

Justin TerAvest
2011-03-10T18:58:21Z

On Thu, Mar 10, 2011 at 10:15 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Thu, Mar 10, 2011 at 10:08:03AM -0800, Justin TerAvest wrote:
>
> [..]
>> > I don't like to increase size of page_cgroup but I think you can record
>> > information without increasing size of page_cgroup.
>> >
>> > A) As Andrea did, encode it to pc->flags.
>> >   But I'm afraid that there is a racy case because memory cgroup uses some
>> >   test_and_set() bits.
>> > B) I wonder why the information cannot be recorded in page->private.
>> >   When page has buffers, you can record the information to buffer struct.
>> >   About swapio (if you take care of), you can record information to bio.
>>
>> Hi Kame,
>>
>> I'm concerned that by using something like buffer_heads stored in
>> page->private, we will only be supported on some filesystems and not
>> others. In addition, I'm not sure if all filesystems attach buffer
>> heads at the same time; if page->private is modified in the flusher
>> thread, we might not be able to determine the thread that dirtied the
>> page in the first place.
>
> I think the person who dirtied the page can store the information in
> page->private (assuming buffer heads were not generated) and if flusher
> thread later ends up generating buffer heads and ends up modifying
> page->private, this can be copied in buffer heads?

This scares me a bit.

As I understand it, fs/ code expects total ownership of page->private.
This adds a responsibility for every user to copy the data through and
store it in the buffer head (or anything else). btrfs seems to do
something entirely different in some cases and store a different kind
of value.

I don't know that it's right to add the burden to copy the original
value to everything that wants to use page->private.

>
> Thanks
> Vivek
>