On Thursday 05 September 2002 14:34, Christian Ehrhardt wrote:
> @@ -455,7 +458,7 @@
> } else {
> /* failed to drop the buffers so stop here */
> UnlockPage(page);
> - page_cache_release(page);
> + put_page(page);
>
> spin_lock(&pagemap_lru_lock);
> continue;
>
> looks a bit suspicious. put_page is not allowed if the page is still
> on the lru and there is no other reference to it. As we don't hold any
> locks between UnlockPage and put_page there is no formal guarantee that
> the above condition is met. I don't have another path that could race
> with this one though and chances are that there actually is none.
The corresponding get_page is just above, you must have overlooked it.
Besides that, we have a promise that the page still has buffers, worth
another count, and the page will never be freed here. That's fragile
though, and this particular piece of code can no doubt be considerably
simplified, while improving robustness and efficiency at the same time.
But that goes beyond the scope of this patch.
The whole try_to_free_buffers path is hairy, scary and disgusting. It
could use some critical analysis, if anybody has time.
-- Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Sep 07 2002 - 22:00:25 EST