Re: [rfc][patch] mm, fs: warn on missing address space operations

[Christoph Hellwig]

> @@ -2472,7 +2472,14 @@ int try_to_release_page(struct page *pag
>  
>  	if (mapping && mapping->a_ops->releasepage)
>  		return mapping->a_ops->releasepage(page, gfp_mask);
> -	return try_to_free_buffers(page);
> +	else {
> +		static bool warned = false;
> +		if (!warned) {
> +			warned = true;
> +			print_symbol("address_space_operations %s missing releasepage method. Use try_to_free_buffers.\n", (unsigned long)page->mapping->a_ops);
> +		}
> +		return try_to_free_buffers(page);
> +	}

I don't think this is correct.  We currently also call
try_to_free_buffers if the page does not have a mapping, and from
conversations with Andrew long time ago that case actually does seem to
be nessecary due to behaviour in ext3/jbd.  So you really should
only warn if there is a mapping to start with.  In fact your code will
dereference a potential NULL pointer in that case.

And as others said, this patch only makes sense after the existing
filesystems are updated to fill out all methods, and for the case
of try_to_free_buffers and set_page_dirty until we have suitable
and well-named default operations available.

Btw, any reason this doesn't use the %pf specifier to printk
instead of dragging in print_symbol?  Even better would
be to just print the fs type from mapping->host->i_sb->s_type->name.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/