Re: [PATCH] Don't map random pages if swapoff errors
Hello! > Getting an error there is all the more reason to proceed > with the swapoff, not to give up and break out of it. Yes, from this viewpoint more reasonable approach would be to untie corresponding ptes from swap entry and mark them as invalid to trigger fault on access. Not even tried simply because it is definitely not that thing, which we needed. We used this for process migration and for that purpose we really need to know when swapoff() fails ASAP to abort migration, to kill processes which got invalid pages and to resume original copy. Obviously, delayed fault is absolutely inappropriate for this particular purpose. Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Don't map random pages if swapoff errors
On Fri, 19 Jan 2007, Alexey Dobriyan wrote: > From: Alexey Kuznetsov <[EMAIL PROTECTED]> > > If read failed we cannot map not-uptodate page to user space. Good point. > Actually, we are in serious troubles, we do not even know what > process to kill. True, though we don't really want to kill anything yet: the process may never need that page again. Better to let it continue until it exits, or hits Kirill's check in do_swap_page. But sure, that's not going to happen without us making some change here. > So, the only variant remains: to stop swapoff() > and allow someone to kill processes to zap invalid pages. Simple as it is, no, I don't like this patch at all. Getting an error there is all the more reason to proceed with the swapoff, not to give up and break out of it. Let me think a little. CC'ed Richard, since he's also interested in bad swap, and this reminds me to look at his patches (though he's been concerned with when the writeout fails, rather than when the readin fails). Hugh > > Signed-off-by: Alexey Kuznetsov <[EMAIL PROTECTED]> > Signed-off-by: Alexey Dobriyan <[EMAIL PROTECTED]> > --- > > mm/swapfile.c | 13 + > 1 file changed, 13 insertions(+) > > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -766,6 +766,19 @@ static int try_to_unuse(unsigned int typ > lock_page(page); > wait_on_page_writeback(page); > > + /* If read failed we cannot map not-uptodate page to > + * user space. Actually, we are in serious troubles, > + * we do not even know what process to kill. So, the only > + * variant remains: to stop swapoff() and allow someone > + * to kill processes to zap invalid pages. > + */ > + if (unlikely(!PageUptodate(page))) { > + unlock_page(page); > + page_cache_release(page); > + retval = -EIO; > + break; > + } > + > /* >* Remove all references to entry. >* Whenever we reach init_mm, there's no address space - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Don't map random pages if swapoff errors
On Fri, 19 Jan 2007, Alexey Dobriyan wrote: From: Alexey Kuznetsov [EMAIL PROTECTED] If read failed we cannot map not-uptodate page to user space. Good point. Actually, we are in serious troubles, we do not even know what process to kill. True, though we don't really want to kill anything yet: the process may never need that page again. Better to let it continue until it exits, or hits Kirill's check in do_swap_page. But sure, that's not going to happen without us making some change here. So, the only variant remains: to stop swapoff() and allow someone to kill processes to zap invalid pages. Simple as it is, no, I don't like this patch at all. Getting an error there is all the more reason to proceed with the swapoff, not to give up and break out of it. Let me think a little. CC'ed Richard, since he's also interested in bad swap, and this reminds me to look at his patches (though he's been concerned with when the writeout fails, rather than when the readin fails). Hugh Signed-off-by: Alexey Kuznetsov [EMAIL PROTECTED] Signed-off-by: Alexey Dobriyan [EMAIL PROTECTED] --- mm/swapfile.c | 13 + 1 file changed, 13 insertions(+) --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -766,6 +766,19 @@ static int try_to_unuse(unsigned int typ lock_page(page); wait_on_page_writeback(page); + /* If read failed we cannot map not-uptodate page to + * user space. Actually, we are in serious troubles, + * we do not even know what process to kill. So, the only + * variant remains: to stop swapoff() and allow someone + * to kill processes to zap invalid pages. + */ + if (unlikely(!PageUptodate(page))) { + unlock_page(page); + page_cache_release(page); + retval = -EIO; + break; + } + /* * Remove all references to entry. * Whenever we reach init_mm, there's no address space - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Don't map random pages if swapoff errors
Hello! Getting an error there is all the more reason to proceed with the swapoff, not to give up and break out of it. Yes, from this viewpoint more reasonable approach would be to untie corresponding ptes from swap entry and mark them as invalid to trigger fault on access. Not even tried simply because it is definitely not that thing, which we needed. We used this for process migration and for that purpose we really need to know when swapoff() fails ASAP to abort migration, to kill processes which got invalid pages and to resume original copy. Obviously, delayed fault is absolutely inappropriate for this particular purpose. Alexey - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/