Commit 28e211700a81b0a934b6c7a4b8e7dda843634d2f
Committed by
Felix Blyakher
1 parent
5f79ed685f
Exists in
master
and in
7 other branches
xfs: fix getbmap vs mmap deadlock
xfs_getbmap (or rather the formatters called by it) copy out the getbmap structures under the ilock, which can deadlock against mmap. This has been reported via bugzilla a while ago (#717) and has recently also shown up via lockdep. So allocate a temporary buffer to format the kernel getbmap structures into and then copy them out after dropping the locks. A little problem with this is that we limit the number of extents we can copy out by the maximum allocation size, but I see no real way around that. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@sandeen.net> Reviewed-by: Felix Blyakher <felixb@sgi.com> Signed-off-by: Felix Blyakher <felixb@sgi.com>
Showing 1 changed file with 35 additions and 17 deletions Side-by-side Diff
fs/xfs/xfs_bmap.c
... | ... | @@ -5890,12 +5890,13 @@ |
5890 | 5890 | int nexleft; /* # of user extents left */ |
5891 | 5891 | int subnex; /* # of bmapi's can do */ |
5892 | 5892 | int nmap; /* number of map entries */ |
5893 | - struct getbmapx out; /* output structure */ | |
5893 | + struct getbmapx *out; /* output structure */ | |
5894 | 5894 | int whichfork; /* data or attr fork */ |
5895 | 5895 | int prealloced; /* this is a file with |
5896 | 5896 | * preallocated data space */ |
5897 | 5897 | int iflags; /* interface flags */ |
5898 | 5898 | int bmapi_flags; /* flags for xfs_bmapi */ |
5899 | + int cur_ext = 0; | |
5899 | 5900 | |
5900 | 5901 | mp = ip->i_mount; |
5901 | 5902 | iflags = bmv->bmv_iflags; |
... | ... | @@ -5971,6 +5972,13 @@ |
5971 | 5972 | return XFS_ERROR(EINVAL); |
5972 | 5973 | bmvend = bmv->bmv_offset + bmv->bmv_length; |
5973 | 5974 | |
5975 | + | |
5976 | + if (bmv->bmv_count > ULONG_MAX / sizeof(struct getbmapx)) | |
5977 | + return XFS_ERROR(ENOMEM); | |
5978 | + out = kmem_zalloc(bmv->bmv_count * sizeof(struct getbmapx), KM_MAYFAIL); | |
5979 | + if (!out) | |
5980 | + return XFS_ERROR(ENOMEM); | |
5981 | + | |
5974 | 5982 | xfs_ilock(ip, XFS_IOLOCK_SHARED); |
5975 | 5983 | if (whichfork == XFS_DATA_FORK && !(iflags & BMV_IF_DELALLOC)) { |
5976 | 5984 | if (ip->i_delayed_blks || ip->i_size > ip->i_d.di_size) { |
5977 | 5985 | |
5978 | 5986 | |
5979 | 5987 | |
5980 | 5988 | |
5981 | 5989 | |
5982 | 5990 | |
5983 | 5991 | |
... | ... | @@ -6025,39 +6033,39 @@ |
6025 | 6033 | ASSERT(nmap <= subnex); |
6026 | 6034 | |
6027 | 6035 | for (i = 0; i < nmap && nexleft && bmv->bmv_length; i++) { |
6028 | - int full = 0; /* user array is full */ | |
6029 | - | |
6030 | - out.bmv_oflags = 0; | |
6036 | + out[cur_ext].bmv_oflags = 0; | |
6031 | 6037 | if (map[i].br_state == XFS_EXT_UNWRITTEN) |
6032 | - out.bmv_oflags |= BMV_OF_PREALLOC; | |
6038 | + out[cur_ext].bmv_oflags |= BMV_OF_PREALLOC; | |
6033 | 6039 | else if (map[i].br_startblock == DELAYSTARTBLOCK) |
6034 | - out.bmv_oflags |= BMV_OF_DELALLOC; | |
6035 | - out.bmv_offset = XFS_FSB_TO_BB(mp, map[i].br_startoff); | |
6036 | - out.bmv_length = XFS_FSB_TO_BB(mp, map[i].br_blockcount); | |
6037 | - out.bmv_unused1 = out.bmv_unused2 = 0; | |
6040 | + out[cur_ext].bmv_oflags |= BMV_OF_DELALLOC; | |
6041 | + out[cur_ext].bmv_offset = | |
6042 | + XFS_FSB_TO_BB(mp, map[i].br_startoff); | |
6043 | + out[cur_ext].bmv_length = | |
6044 | + XFS_FSB_TO_BB(mp, map[i].br_blockcount); | |
6045 | + out[cur_ext].bmv_unused1 = 0; | |
6046 | + out[cur_ext].bmv_unused2 = 0; | |
6038 | 6047 | ASSERT(((iflags & BMV_IF_DELALLOC) != 0) || |
6039 | 6048 | (map[i].br_startblock != DELAYSTARTBLOCK)); |
6040 | 6049 | if (map[i].br_startblock == HOLESTARTBLOCK && |
6041 | 6050 | whichfork == XFS_ATTR_FORK) { |
6042 | 6051 | /* came to the end of attribute fork */ |
6043 | - out.bmv_oflags |= BMV_OF_LAST; | |
6052 | + out[cur_ext].bmv_oflags |= BMV_OF_LAST; | |
6044 | 6053 | goto out_free_map; |
6045 | 6054 | } |
6046 | 6055 | |
6047 | - if (!xfs_getbmapx_fix_eof_hole(ip, &out, prealloced, | |
6048 | - bmvend, map[i].br_startblock)) | |
6056 | + if (!xfs_getbmapx_fix_eof_hole(ip, &out[cur_ext], | |
6057 | + prealloced, bmvend, | |
6058 | + map[i].br_startblock)) | |
6049 | 6059 | goto out_free_map; |
6050 | 6060 | |
6051 | - /* format results & advance arg */ | |
6052 | - error = formatter(&arg, &out, &full); | |
6053 | - if (error || full) | |
6054 | - goto out_free_map; | |
6055 | 6061 | nexleft--; |
6056 | 6062 | bmv->bmv_offset = |
6057 | - out.bmv_offset + out.bmv_length; | |
6063 | + out[cur_ext].bmv_offset + | |
6064 | + out[cur_ext].bmv_length; | |
6058 | 6065 | bmv->bmv_length = |
6059 | 6066 | max_t(__int64_t, 0, bmvend - bmv->bmv_offset); |
6060 | 6067 | bmv->bmv_entries++; |
6068 | + cur_ext++; | |
6061 | 6069 | } |
6062 | 6070 | } while (nmap && nexleft && bmv->bmv_length); |
6063 | 6071 | |
... | ... | @@ -6067,6 +6075,16 @@ |
6067 | 6075 | xfs_iunlock_map_shared(ip, lock); |
6068 | 6076 | out_unlock_iolock: |
6069 | 6077 | xfs_iunlock(ip, XFS_IOLOCK_SHARED); |
6078 | + | |
6079 | + for (i = 0; i < cur_ext; i++) { | |
6080 | + int full = 0; /* user array is full */ | |
6081 | + | |
6082 | + /* format results & advance arg */ | |
6083 | + error = formatter(&arg, &out[i], &full); | |
6084 | + if (error || full) | |
6085 | + break; | |
6086 | + } | |
6087 | + | |
6070 | 6088 | return error; |
6071 | 6089 | } |
6072 | 6090 |