вт, 26 янв. 2021 г. в 15:57, ronnie sahlberg <ronniesahlberg@xxxxxxxxx>: > > On Wed, Jan 27, 2021 at 9:34 AM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: > > > > пн, 25 янв. 2021 г. в 09:07, Shyam Prasad N <nspmangalore@xxxxxxxxx>: > > > > > > From my readings so far, -ERESTARTSYS assumes that the syscall is idempotent. > > > Can we safely make such assumptions for all VFS file operations? For > > > example, what happens if a close gets restarted, and we already > > > scheduled a delayed close because the original close failed with > > > ERESTARTSYS? > > > > I don't think we can assume all system calls to be idempotent. We > > should probably examine them one-by-one and return -ERESTARTSYS for > > some and -EINTR for the others. > > > > > > > > Also, I ran a quick grep for EINTR, and it looks like __cifs_readv and > > > __cifs_writev return EINTR too. Should those be replaced by > > > ERESTARTSYS too? > > > > > > > They return -EINTR after receiving a kill signal (see > > wait_for_completion_killable around those lines). It doesn't seem > > there is any point in returning -ERESTARTSYS after a kill signal > > anyway, so, I think the code is correct there. > > We have to be careful with -ERESTARTSYS especially in the context of > doing it when signals are pending. > I was just thinking about how signals are cleared and how this might > matter for -EINTR versus -ERESTARTSYS > > As far as I know signals will only become "un"-pending once we hit the > signal handler down in userspace. > So, if we do -ERESTARTSYS because signals are pending, then the kernel > just retries without bouncing down into userspace first > and thus we never hit the signal handler so when we get back to the > if (signal_penging) return -ERESTARTSYS > the signal will still be pending and we risk looping? Signals are handled before restarting a system call. See details here: https://lwn.net/Articles/528935/ So, if signal_pending is true again in a restarted system call then this is another signal that needs to be handled. -- Best regards, Pavel Shilovsky
