On 07/26/12 10:32, Jeff Liu wrote:
This function is called by xfs_seek_data() and xfs_seek_hole() to find
the desired offset from page cache.
Signed-off-by: Jie Liu<jeff.liu@xxxxxxxxxx>
Hopefully, I am not being a pain....
I just noticed that if trylock() failed you return found==0.
Wouldn't it be safer/more correct to assume a page that failed a
try_lock to be data?
+ if (nr_pages == 0) {
+ if (type == HOLE_OFF) {
+ if (coff == *offset)
+ found = true;
is this necessary? wouldn't the next test also cover the above condition?
+ if (coff< endoff) {
+ found = true;
+ *offset = coff;
+ }
+ }
I like informative comments, but some are bit verbose. I will pick on
this one:
+ /*
+ * Page index is out of range, we need to deal with
+ * hole search condition in paticular if that is the
+ * desired type for the lookup.
+ * stepping into the block buffer checkup, it probably
+ * means that there is no page mapped at all in the
+ * specified range to search, so we found a hole.
+ * If we have already done some block buffer checking
+ * and found one or more data buffers before, in this
+ * case, the coff is already updated and it point to
+ * the end of the last data buffer, so the left range
+ * behind it might be a hole. In either case, we will
+ * return the coff to indicate a hole's location because
+ * it must be greater than or equal to the search start.
+ */
just a crude simplification - maybe it is too terse:
/*
* coff is the current offset of the page being tested.
* If the next page index is beyond the extent of interest,
* then we are done searching with the data search is
* false and hole search is true at the last coff.
*/
For holes you are looking for (page->index != coff) for every page, but
in a indirect way. It had me a little confused, but eventually I figured
it out. I am not sure if a doing that comparison directly would overly
complicate the data search path.
Good work.
--Mark.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
