Message ID | 1351177969-893-2-git-send-email-n-horiguchi@ah.jp.nec.com |
---|---|
State | Accepted, archived |
Headers | show |
On Thu 25-10-12 11:12:47, Naoya Horiguchi wrote: > Printing out the information about which file can be affected by a > memory error in generic_error_remove_page() is helpful for user to > estimate the impact of the error. > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > --- > mm/truncate.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git v3.7-rc2.orig/mm/truncate.c v3.7-rc2/mm/truncate.c > index d51ce92..df0c6ab7 100644 > --- v3.7-rc2.orig/mm/truncate.c > +++ v3.7-rc2/mm/truncate.c > @@ -151,14 +151,20 @@ int truncate_inode_page(struct address_space *mapping, struct page *page) > */ > int generic_error_remove_page(struct address_space *mapping, struct page *page) > { > + int ret; > + struct inode *inode = mapping->host; > + This will oops if mapping == NULL. Currently the only caller seems to check beforehand but still, it's better keep the code as robust as it it. > if (!mapping) > return -EINVAL; > /* > * Only punch for normal data pages for now. > * Handling other types like directories would need more auditing. > */ > - if (!S_ISREG(mapping->host->i_mode)) > + if (!S_ISREG(inode->i_mode)) > return -EIO; > + pr_info("MCE %#lx: file info pgoff:%lu, inode:%lu, dev:%s\n", > + page_to_pfn(page), page_index(page), > + inode->i_ino, inode->i_sb->s_id); > return truncate_inode_page(mapping, page); > } > EXPORT_SYMBOL(generic_error_remove_page); Otherwise the patch looks OK. Honza
Hi Jan, Thank you for taking time for the review. On Thu, Oct 25, 2012 at 09:32:49PM +0200, Jan Kara wrote: > On Thu 25-10-12 11:12:47, Naoya Horiguchi wrote: > > Printing out the information about which file can be affected by a > > memory error in generic_error_remove_page() is helpful for user to > > estimate the impact of the error. > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > --- > > mm/truncate.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git v3.7-rc2.orig/mm/truncate.c v3.7-rc2/mm/truncate.c > > index d51ce92..df0c6ab7 100644 > > --- v3.7-rc2.orig/mm/truncate.c > > +++ v3.7-rc2/mm/truncate.c > > @@ -151,14 +151,20 @@ int truncate_inode_page(struct address_space *mapping, struct page *page) > > */ > > int generic_error_remove_page(struct address_space *mapping, struct page *page) > > { > > + int ret; > > + struct inode *inode = mapping->host; > > + > This will oops if mapping == NULL. Currently the only caller seems to > check beforehand but still, it's better keep the code as robust as it it. OK. Adding a comment about it will be helpful for that purpose. Thanks, Naoya -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git v3.7-rc2.orig/mm/truncate.c v3.7-rc2/mm/truncate.c index d51ce92..df0c6ab7 100644 --- v3.7-rc2.orig/mm/truncate.c +++ v3.7-rc2/mm/truncate.c @@ -151,14 +151,20 @@ int truncate_inode_page(struct address_space *mapping, struct page *page) */ int generic_error_remove_page(struct address_space *mapping, struct page *page) { + int ret; + struct inode *inode = mapping->host; + if (!mapping) return -EINVAL; /* * Only punch for normal data pages for now. * Handling other types like directories would need more auditing. */ - if (!S_ISREG(mapping->host->i_mode)) + if (!S_ISREG(inode->i_mode)) return -EIO; + pr_info("MCE %#lx: file info pgoff:%lu, inode:%lu, dev:%s\n", + page_to_pfn(page), page_index(page), + inode->i_ino, inode->i_sb->s_id); return truncate_inode_page(mapping, page); } EXPORT_SYMBOL(generic_error_remove_page);
Printing out the information about which file can be affected by a memory error in generic_error_remove_page() is helpful for user to estimate the impact of the error. Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> --- mm/truncate.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)