[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANaxB-x8k-c+gkeUJ1LovxSdWHJZggt0POPQxEotmuQvcXeicg@mail.gmail.com>
Date: Thu, 27 Dec 2012 22:40:11 +0400
From: Andrey Wagin <avagin@...il.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Andrew Vagin <avagin@...allels.com>,
Pavel Emelyanov <xemul@...allels.com>,
David Howells <dhowells@...hat.com>,
linux-kernel@...r.kernel.org, criu@...nvz.org,
Cyrill Gorcunov <gorcunov@...nvz.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
linux-fsdevel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Michael Kerrisk <mtk.manpages@...il.com>
Subject: Re: [CRIU] [PATCH 1/4] signalfd: add ability to return siginfo in a
raw format
2012/12/27 Oleg Nesterov <oleg@...hat.com>:
> On 12/27, Andrey Wagin wrote:
>>
>> On Wed, Dec 26, 2012 at 05:31:12PM +0100, Oleg Nesterov wrote:
>> >
>> > So I think we should not use llseek. But, probably we can rely on pread() ?
>> > This way we can avoid the problem above, and this looks even simpler.
>>
>> Yes. It is a good idea. A new patch is attached to this email. I
>> implemented pread for signalfd and fixed all your previous comments.
>>
...
> I think it would be better to pass ppos, not *ppos, because ...
>
>> + if (*ppos)
>> + (*ppos)++;
>
> in this case we can update *ppos in signalfd_peek() and simplify the
> code a bit.
>
> Compared to the previous version it is "safe" to change *ppos even if
> copy_to_user() fails, *ppos will be "lost" anyway after we return.
Yes. But ppos is updated, because pread() reads all siginfo-s from a queue,
which fit in a buffer starting with the initial ppos.
>
>> @@ -321,6 +372,7 @@ SYSCALL_DEFINE4(signalfd4, int, ufd, sigset_t __user *, user_mask,
>> }
>>
>> file->f_flags |= flags & SFD_RAW;
>> + file->f_mode |= FMODE_PREAD;
>>
>> fd_install(ufd, file);
>
> Hmm. Looks like it is based on other patches I didnt see...
>
> But I don't understand how FMODE_PREAD connects to this patch, we need
> this flag set even for regular sys_read() ???
It doesn't need for sys_read(), but this patch is about pread() and
sys_pread() checks it.
SYSCALL_DEFINE(pread64)(unsigned int fd, char __user *buf,
size_t count, loff_t pos)
{
....
if (f.file) {
ret = -ESPIPE;
if (f.file->f_mode & FMODE_PREAD)
ret = vfs_read(f.file, buf, count, &pos);
>
>> +#define SIGNALFD_SHARED_OFFSET (1LL << 62)
>
> OK... this assumes we are not going to add more SIGNAL_*_OFFSET's, and
> probably this is true...
>
> Oleg.
>
Thanks for review.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists