> Yes. From the changelog:
>
> Of course, this means we read pending/blocked/etc nonatomically,
> but I hope this is OK for fs/proc.
>
> But I don't think the returned data could be "really" inconsistent
> from the /bin/ps pov. Yes, it is possible that, say, some signal is
> seen as both pending and ignored without ->siglock. Or we can report
> user->sigpending != 0 while pending/shpending are empty.
>
> But this looks harmless to me. We never guaranteed /proc/pid/status
> can't report the "intermediate" state, and I don't think we can
> confuse the user-space.
>
> Do you agree? Or do you think this can make problems ?
I'm not so sure. Operations like sigprocmask and sigaction really have
always been entirely atomic from the userland perspective before. Now it
becomes possible to read from /proc e.g. a blocked set that never existed
as such (one word updated by sigprocmask but not yet the next word).
Thanks,
Roland
Roland McGrath <[email protected]> wrote:
> I'm not so sure. Operations like sigprocmask and sigaction really have
> always been entirely atomic from the userland perspective before. Now it
> becomes possible to read from /proc e.g. a blocked set that never existed
> as such (one word updated by sigprocmask but not yet the next word).
If you have a small userspace buffer, that was previously possible too.
David
On 04/09, Roland McGrath wrote:
>
> > Yes. From the changelog:
> >
> > Of course, this means we read pending/blocked/etc nonatomically,
> > but I hope this is OK for fs/proc.
> >
> > But I don't think the returned data could be "really" inconsistent
> > from the /bin/ps pov. Yes, it is possible that, say, some signal is
> > seen as both pending and ignored without ->siglock. Or we can report
> > user->sigpending != 0 while pending/shpending are empty.
> >
> > But this looks harmless to me. We never guaranteed /proc/pid/status
> > can't report the "intermediate" state, and I don't think we can
> > confuse the user-space.
> >
> > Do you agree? Or do you think this can make problems ?
>
> I'm not so sure. Operations like sigprocmask and sigaction really have
> always been entirely atomic from the userland perspective before. Now it
> becomes possible to read from /proc e.g. a blocked set that never existed
> as such (one word updated by sigprocmask but not yet the next word).
Yes, /proc/pid/status can report the intermediate state, I even sent
the updated changelog to document this.
But if you are not sure this is OK, I am worried. Do you think we should
drop this patch? If yes, I won't argue.
Oleg.
> Yes, /proc/pid/status can report the intermediate state, I even sent
> the updated changelog to document this.
>
> But if you are not sure this is OK, I am worried. Do you think we should
> drop this patch? If yes, I won't argue.
I'm not dead-set against it, but I am hesitant. My inclination is not to
remove any previous userland atomicity guarantees with regard to observable
signal state in any form. At least, don't do that in part of a whole
cleanup flurry where it is intermixed with lots of changes that really are
pure cleanup with absolutely no userland-observable change. If it really
helps to fragment what was atomic before, then we can consider it. But
let's not be in a hurry.
David mentioned that users who do multiple reads due to using tiny buffers
already don't get atomic sampling. That is certainly true but I don't
think it's relevant. It is completely reliable that you can easily
allocate a buffer big enough to get all the Sig* fields on the first read,
and any user program that might care about the coherence of the data,
by definition, is already doing that.
Thanks,
Roland
OK. Andrew, please drop
proc-make-collect_sigign_sigcatch-rcu-safe.patch
proc-make-task_sig-lockless.patch
On 04/12, Roland McGrath wrote:
>
> > Yes, /proc/pid/status can report the intermediate state, I even sent
> > the updated changelog to document this.
> >
> > But if you are not sure this is OK, I am worried. Do you think we should
> > drop this patch? If yes, I won't argue.
>
> I'm not dead-set against it, but I am hesitant. My inclination is not to
> remove any previous userland atomicity guarantees with regard to observable
> signal state in any form.
OK. Not that I really understand why do we need atomicity, but OK.
I was going to remove ->siglock from /fs/proc/ completely (except
do_io_accounting), but given that nobody replied to do_task_stat patches
this will not happen soon.
> At least, don't do that in part of a whole
> cleanup flurry where it is intermixed with lots of changes that really are
> pure cleanup with absolutely no userland-observable change.
OK. Anyway, these changes are simple, we can reconsider them later.
Oleg.