xfs
[Top] [All Lists]

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

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH 10/11] xfs: always return with the iolock held from xfs_file_aio_write_checks
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Fri, 20 Jan 2012 20:51:22 +0800
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20120117201817.GG16581@xxxxxxx>
Organization: Oracle
References: <20111218200003.557507716@xxxxxxxxxxxxxxxxxxxxxx> <20111218200132.483776880@xxxxxxxxxxxxxxxxxxxxxx> <20120117201817.GG16581@xxxxxxx>
Reply-to: jeff.liu@xxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.18) Gecko/20110617 Thunderbird/3.1.11
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@xxxxxxxxxx>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> Looks good.
> Reviewed-by: Ben Myers <bpm@xxxxxxx>
> 
>>
>> ---
>>  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

<Prev in Thread] Current Thread [Next in Thread>