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? > > -- > Best regards, > Pavel Shilovsky