On Thu, Nov 03, 2011 at 11:37:03AM +0530, Amit Sahrawat wrote:
> This is needed for long term kernel 2.6.35.14.
> Please let me know for any changes/suggestions.
It sounds like a fine candidate to backport, although the context
differs a lot from the actual changes commited to mainline. A few
comments below:
> they are overlapping IO and the result of concurrent overlapping IOs
> is undefined - the result of either IO is a valid result so we let
> them race. Hence we only penalise unaligned IO, which already has a
> major overhead compared to aligned IO so this isn't a major problem.
>
You probably should keep the original Signoff and reviewed-by tags,
and add your editor note on the top into [ ] brackets.
> + if (!need_i_mutex && ( unaligned_io || mapping->nrpages || pos >
> ip->i_size)) {
no space after the opening brace please, and split overly-long lines
into two:
if (!need_i_mutex &&
(unaligned_io || mapping->nrpages || pos > ip->i_size)) {
> if (need_i_mutex) {
> - /* demote the lock now the cached pages are gone */
> - xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
> + if (unaligned_io)
> + xfs_ioend_wait(ip);
> + /* demote the lock now the cached pages are gone if we can */
> + else {
> + xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
> + iolock = XFS_IOLOCK_SHARED;
> + }
Please use the comment that was used upstream here:
/*
* If we are doing unaligned IO, wait for all other IO
* to drain, otherwise demote the lock if we had to
* flush cached pages.
*/
> mutex_unlock(&inode->i_mutex);
>
> - iolock = XFS_IOLOCK_SHARED;
> need_i_mutex = 0;
You also need to make the i_mutex unlock and need_i_mutex update
conditional here, otherwise you still serialize all O_DIRECT writes.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
