Re: [PATCH 10/11] xfs: always return with the iolock held from xfs_file_aio_write_checks

[Jeff Liu <jeff.liu@oracle.com>]

On 01/18/2012 04:18 AM, Ben Myers wrote:

> On Sun, Dec 18, 2011 at 03:00:13PM -0500, Christoph Hellwig wrote:
>> While xfs_iunlock is fine with 0 lockflags the calling conventions are much
>> cleaner if xfs_file_aio_write_checks never returns without the iolock held.
>>
>> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks good.
> Reviewed-by: Ben Myers <bpm@sgi.com>
> 
>>
>> ---
>>  fs/xfs/xfs_file.c |    7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> Index: xfs/fs/xfs/xfs_file.c
>> ===================================================================
>> --- xfs.orig/fs/xfs/xfs_file.c	2011-12-07 12:46:31.343897882 +0100
>> +++ xfs/fs/xfs/xfs_file.c	2011-12-07 12:48:33.309903801 +0100
>> @@ -636,7 +636,9 @@ out_lock:
>>  /*
>>   * Common pre-write limit and setup checks.
>>   *
>> - * Returns with iolock held according to @iolock.
>> + * Called with the iolocked held either shared and exclusive according to
>> + * @iolock, and returns with it held.  Might upgrade the iolock to exclusive
>> + * if called for a direct write beyond i_size.
>>   */
>>  STATIC ssize_t
>>  xfs_file_aio_write_checks(
>> @@ -653,8 +655,7 @@ xfs_file_aio_write_checks(
>>  restart:
>>  	error = generic_write_checks(file, pos, count, S_ISBLK(inode->i_mode));
>>  	if (error) {
>> -		xfs_rw_iunlock(ip, XFS_ILOCK_EXCL | *iolock);
>> -		*iolock = 0;
>> +		xfs_rw_iunlock(ip, XFS_ILOCK_EXCL);
>>  		return error;
>>  	}

Haha, I lucky to saw this patch, since I have triggered an Oops on 3.2.0-rc6 without it just now.

FYI, test code:

#define _GNU_SOURCE
#define _LARGEFILE64_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <ctype.h>
#include <errno.h>
#include <linux/falloc.h>

int
main(int argc, char **argv)
{
	off64_t ret = 0;
	char *path;
	int fd;

	if (argc != 2) {
		fprintf(stdout, "Usage: %s filename\n", argv[0]);
		return 1;
	}

	path = argv[1];		
	fd = open(path, O_WRONLY|O_CREAT|O_TRUNC, 0644);
	if (fd < 0) {
		perror("open");
		return fd;
	}

	ret = fallocate(fd, 0, 0, 1);
	if (ret < 0) {
		fprintf(stderr, "fallocate failed due to %s\n", strerror(errno));
		return ret;
	}

	ret = lseek64(fd, (off64_t)2947483650, SEEK_SET);
	if (ret < 0) {
		perror("lseek");
		return ret;
	}

	write(fd, "abc", 3);

	ret = lseek64(fd, (off64_t)2997483650, SEEK_SET);
	if (ret < 0) {
		perror("lseek");
		return ret;
	}

	write(fd, "abc", 3);

	close(fd);

	return ret;
}

[  135.592793] XFS: Assertion failed: lock_flags != 0, file: fs/xfs/xfs_iget.c, line: 652
[  135.592836] ------------[ cut here ]------------
[  135.596010] kernel BUG at fs/xfs/xfs_message.c:101!
[  135.596010] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
[  135.596010] Modules linked in: xfs(O) binfmt_misc kvm_intel kvm ocfs2_dlmfs ocfs2_stack_o2cb ocfs2_dlm ocfs2_nodemanager ocfs2_stackglue configfs xt_TCPMSS xt_tcpmss xt_tcpudp iptable_mangle pppoe pppox nfsd exportfs nfs ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 lockd nf_conntrack nf_defrag_ipv4 parport_pc ppdev ip_tables x_tables fscache auth_rpcgss bridge nfs_acl sunrpc stp snd_hda_codec_analog arc4 snd_hda_intel snd_hda_codec snd_hwdep iwl4965 snd_pcm iwl_legacy thinkpad_acpi i915 snd_seq_midi snd_rawmidi pcmcia mac80211 joydev snd_seq_midi_event snd_seq drm_kms_helper btrfs snd_timer snd_seq_device zlib_deflate libcrc32c psmouse btusb yenta_socket pcmcia_rsrc drm cfg80211 bluetooth serio_raw pcmcia_core snd tpm_tis tpm tpm_bios nvram soundcore snd_page_alloc lp i2c_algo_bit parport video ext4 mbcache jbd2 firewire_ohci firewire_core crc_itu_t ahci libahci e1000e
[  135.596010] 
[  135.596010] Pid: 2313, comm: faa Tainted: G        W  O 3.2.0-rc6 #9 LENOVO 7661D43/7661D43
[  135.596010] EIP: 0060:[<f94ae5e8>] EFLAGS: 00010246 CPU: 1
[  135.596010] EIP is at assfail+0x47/0x57 [xfs]
[  135.596010] EAX: 00000060 EBX: e6674400 ECX: 00000000 EDX: 00000073
[  135.596010] ESI: 00000000 EDI: 00000000 EBP: e8d59e34 ESP: e8d59e20
[  135.596010]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  135.596010] Process faa (pid: 2313, ti=e8d58000 task=e919c6c0 task.ti=e8d58000)
[  135.596010] Stack:
[  135.596010]  00000000 f956b082 f956abbb f956a97f 0000028c e8d59e50 f94a3027 e8d59e4c
[  135.596010]  f949c2bd e6674400 00000000 f956171c e8d59e60 f949c2bd afaf0802 00000000
[  135.596010]  e8d59ed4 f949eb38 afaf0802 00000000 00000003 e8d59eb8 e8d59ec4 000021ef
[  135.596010] Call Trace:
[  135.596010]  [<f94a3027>] xfs_iunlock+0xe6/0x2c0 [xfs]
[  135.596010]  [<f949c2bd>] ? xfs_rw_iunlock+0x21/0x5f [xfs]
[  135.596010]  [<f949c2bd>] xfs_rw_iunlock+0x21/0x5f [xfs]
[  135.596010]  [<f949eb38>] xfs_file_aio_write+0x3cf/0x3e8 [xfs]
[  135.596010]  [<c1215c94>] do_sync_write+0xaa/0x12c
[  135.596010]  [<c12edddc>] ? security_file_permission+0x53/0x65
[  135.596010]  [<c12166a5>] ? rw_verify_area+0x1c4/0x1fe
[  135.596010]  [<c1215bea>] ? wait_on_retry_sync_kiocb+0x8c/0x8c
[  135.596010]  [<c1216af6>] vfs_write+0xf5/0x1a3
[  135.596010]  [<c1218dff>] ? fget_light+0x3e/0x130
[  135.596010]  [<c1216e59>] sys_write+0x6c/0xa9
[  135.596010]  [<c18178b4>] syscall_call+0x7/0xb
[  135.596010] Code: 10 89 54 24 0c 89 44 24 08 c7 44 24 04 82 b0 56 f9 c7 04 24 00 00 00 00 e8 ad fc ff ff 83 05 e0 45 59 f9 01 83 15 e4 45 59 f9 00 <0f> 0b 83 05 e8 45 59 f9 01 83 15 ec 45 59 f9 00 55 89 e5 83 ec 
[  135.596010] EIP: [<f94ae5e8>] assfail+0x47/0x57 [xfs] SS:ESP 0068:e8d59e20
[  135.833048] ---[ end trace 4af94dd273c5d0cb ]---

Call chains:
xfs_file_aio_write->xfs_file_buffered_aio_write->xfs_file_aio_write_checks()->generic_write_checks() failed due to -EFBIG, and the iolock was set to *ZERO* at that time. :-P.

Thanks,
-Jeff

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs