* [PATCH V3 1/1] fs/udf RFC sanitize types, use consistent naming and add kernel doc comments
@ 2017-01-14 17:37 Steve Kenton
2017-01-16 14:54 ` Jan Kara
0 siblings, 1 reply; 2+ messages in thread
From: Steve Kenton @ 2017-01-14 17:37 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, Steve Kenton
Signed-off-by: Steve Kenton <skenton@ou.edu>
---
V2: fix sector_t variable print format
V3: Add long long cast for sector_t 32/64 bit
fs/udf/balloc.c | 334 +++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 234 insertions(+), 100 deletions(-)
diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index e0fd65f..f010caa 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -4,6 +4,17 @@
* PURPOSE
* Block allocation handling routines for the OSTA-UDF(tm) filesystem.
*
+ * Note, ECMA-167 block numbers are unsigned 32-bit values and UDF extents
+ * cannot exceed an unsigned 30-bit byte length so both were traditionally
+ * unsigned 32-bit types despite the use of block_count << bits_per_block
+ * to compute extent_length, which might otherwise risk overflow. But,
+ * UDF filesystems can consist of up to 64K partitions on multiple volumes
+ * and files can have multiple extents resulting in terrabyte+ lengths.
+ * So, use the VFS preferred 64-bit sector_t and loff_t types for block
+ * numbers and file offsets respectively and int for UDF partition number
+ * and extent type as well as extent byte lengths and extent block counts.
+ * All together this makes the UDF code feel similar to other filesystems.
+ *
* COPYRIGHT
* This file is distributed under the terms of the GNU General Public
* License (GPL). Copies of the GPL can be obtained from:
@@ -31,8 +42,12 @@
#define udf_test_bit test_bit_le
#define udf_find_next_one_bit find_next_bit_le
+/*
+ * Read the bitmap from the media - udf_tread() wraps fixed/variable media
+ * to deal with certain peculiar antique optical cartridge devices. Still needed ???
+ */
static int read_block_bitmap(struct super_block *sb,
- struct udf_bitmap *bitmap, unsigned int block,
+ struct udf_bitmap *bitmap, sector_t block,
unsigned long bitmap_nr)
{
struct buffer_head *bh = NULL;
@@ -89,7 +104,16 @@ static inline int load_block_bitmap(struct super_block *sb,
return slot;
}
-static void udf_add_free_space(struct super_block *sb, u16 partition, u32 cnt)
+/**
+ * udf_update_free_space() - Adjust the partition free space available total
+ * @sb: Super block of UDF filesystem mount
+ * @partition: UDF partition to use
+ * @count: Number of blocks to add/subtract
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ * This is just the on-media book keeping for allocate/free performed elsewhere
+ */
+static void udf_update_free_space(struct super_block *sb, int partition, int count)
{
struct udf_sb_info *sbi = UDF_SB(sb);
struct logicalVolIntegrityDesc *lvid;
@@ -98,20 +122,34 @@ static void udf_add_free_space(struct super_block *sb, u16 partition, u32 cnt)
return;
lvid = (struct logicalVolIntegrityDesc *)sbi->s_lvid_bh->b_data;
- le32_add_cpu(&lvid->freeSpaceTable[partition], cnt);
+ le32_add_cpu(&lvid->freeSpaceTable[partition], count);
udf_updated_lvid(sb);
}
+/*
+ * Unallocated/freespace bitmap implementation routines
+ */
+
+/**
+ * udf_bitmap_free_blocks() - Free extent blocks
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space bitmap
+ * @eloc: Extent location/length descriptor
+ * @offset: Block offset from the start of the extent
+ * @count: Number of blocks to free
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ */
static void udf_bitmap_free_blocks(struct super_block *sb,
struct udf_bitmap *bitmap,
- struct kernel_lb_addr *bloc,
- uint32_t offset,
- uint32_t count)
+ struct kernel_lb_addr *eloc,
+ sector_t offset,
+ int count)
{
struct udf_sb_info *sbi = UDF_SB(sb);
struct buffer_head *bh = NULL;
struct udf_part_map *partmap;
- unsigned long block;
+ sector_t block;
unsigned long block_group;
unsigned long bit;
unsigned long i;
@@ -119,17 +157,17 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
unsigned long overflow;
mutex_lock(&sbi->s_alloc_mutex);
- partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];
- if (bloc->logicalBlockNum + count < count ||
- (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
+ partmap = &sbi->s_partmaps[eloc->partitionReferenceNum];
+ if (eloc->logicalBlockNum + count < count ||
+ (eloc->logicalBlockNum + count) > partmap->s_partition_len) {
udf_debug("%d < %d || %d + %d > %d\n",
- bloc->logicalBlockNum, 0,
- bloc->logicalBlockNum, count,
+ eloc->logicalBlockNum, 0,
+ eloc->logicalBlockNum, count,
partmap->s_partition_len);
goto error_return;
}
- block = bloc->logicalBlockNum + offset +
+ block = eloc->logicalBlockNum + offset +
(sizeof(struct spaceBitmapDesc) << 3);
do {
@@ -156,7 +194,7 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
((char *)bh->b_data)[(bit + i) >> 3]);
}
}
- udf_add_free_space(sb, sbi->s_partition, count);
+ udf_update_free_space(sb, sbi->s_partition, count);
mark_buffer_dirty(bh);
if (overflow) {
block += count;
@@ -168,25 +206,38 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
mutex_unlock(&sbi->s_alloc_mutex);
}
+/**
+ * udf_bitmap_prealloc_blocks() - Allocate partition blocks
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space bitmap
+ * @partition: UDF partition to use
+ * @first_block: Start of a free, or for write once media unrecorded, extent
+ * @count: Maximum number of blocks to try and allocate
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ *
+ * Return: Number of blocks allocated on success or 0 on failure
+ */
static int udf_bitmap_prealloc_blocks(struct super_block *sb,
struct udf_bitmap *bitmap,
- uint16_t partition, uint32_t first_block,
- uint32_t block_count)
+ int partition, sector_t first_block,
+ int count)
{
struct udf_sb_info *sbi = UDF_SB(sb);
int alloc_count = 0;
- int bit, block, block_group, group_start;
+ sector_t block;
+ int bit, block_group, group_start;
int nr_groups, bitmap_nr;
struct buffer_head *bh;
- __u32 part_len;
+ sector_t part_len;
mutex_lock(&sbi->s_alloc_mutex);
part_len = sbi->s_partmaps[partition].s_partition_len;
if (first_block >= part_len)
goto out;
- if (first_block + block_count > part_len)
- block_count = part_len - first_block;
+ if (first_block + count > part_len)
+ count = part_len - first_block;
do {
nr_groups = udf_compute_nr_groups(sb, partition);
@@ -201,33 +252,43 @@ static int udf_bitmap_prealloc_blocks(struct super_block *sb,
bit = block % (sb->s_blocksize << 3);
- while (bit < (sb->s_blocksize << 3) && block_count > 0) {
+ while (bit < (sb->s_blocksize << 3) && count > 0) {
if (!udf_clear_bit(bit, bh->b_data))
goto out;
- block_count--;
+ count--;
alloc_count++;
bit++;
block++;
}
mark_buffer_dirty(bh);
- } while (block_count > 0);
+ } while (count > 0);
out:
- udf_add_free_space(sb, partition, -alloc_count);
+ udf_update_free_space(sb, partition, -alloc_count);
mutex_unlock(&sbi->s_alloc_mutex);
return alloc_count;
}
-static int udf_bitmap_new_block(struct super_block *sb,
- struct udf_bitmap *bitmap, uint16_t partition,
- uint32_t goal, int *err)
+/**
+ * udf_bitmap_new_block() - Allocate a partition block close to goal
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space bitmap
+ * @partition: UDF partition to use
+ * @goal: The block on the partition to which proximity is desired
+ * @err: If not successful this is the error that caused the failure
+ *
+ * Return: Partition block number on success or 0 and set *err on failure
+ */
+static sector_t udf_bitmap_new_block(struct super_block *sb,
+ struct udf_bitmap *bitmap, int partition,
+ sector_t goal, int *err)
{
struct udf_sb_info *sbi = UDF_SB(sb);
- int newbit, bit = 0, block, block_group, group_start;
+ int newbit, bit = 0, block_group, group_start;
int end_goal, nr_groups, bitmap_nr, i;
struct buffer_head *bh = NULL;
char *ptr;
- int newblock = 0;
+ sector_t block, newblock = 0;
*err = -ENOSPC;
mutex_lock(&sbi->s_alloc_mutex);
@@ -332,7 +393,7 @@ static int udf_bitmap_new_block(struct super_block *sb,
mark_buffer_dirty(bh);
- udf_add_free_space(sb, partition, -1);
+ udf_update_free_space(sb, partition, -1);
mutex_unlock(&sbi->s_alloc_mutex);
*err = 0;
return newblock;
@@ -343,37 +404,51 @@ static int udf_bitmap_new_block(struct super_block *sb,
return 0;
}
+/*
+ * Unallocated/freespace table implementation routines
+ */
+
+/**
+ * udf_table_free_blocks() - Free extent blocks
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space table
+ * @eloc: Extent location/length descriptor
+ * @offset: Block offset from the start of the extent
+ * @count: Number of blocks to free
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ */
static void udf_table_free_blocks(struct super_block *sb,
struct inode *table,
- struct kernel_lb_addr *bloc,
- uint32_t offset,
- uint32_t count)
+ struct kernel_lb_addr *eloc,
+ sector_t offset,
+ int count)
{
struct udf_sb_info *sbi = UDF_SB(sb);
struct udf_part_map *partmap;
- uint32_t start, end;
- uint32_t elen;
- struct kernel_lb_addr eloc;
+ sector_t start, end;
+ int elen;
+ struct kernel_lb_addr bloc;
struct extent_position oepos, epos;
- int8_t etype;
+ int etype;
struct udf_inode_info *iinfo;
mutex_lock(&sbi->s_alloc_mutex);
- partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];
- if (bloc->logicalBlockNum + count < count ||
- (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
+ partmap = &sbi->s_partmaps[eloc->partitionReferenceNum];
+ if (eloc->logicalBlockNum + count < count ||
+ (eloc->logicalBlockNum + count) > partmap->s_partition_len) {
udf_debug("%d < %d || %d + %d > %d\n",
- bloc->logicalBlockNum, 0,
- bloc->logicalBlockNum, count,
+ eloc->logicalBlockNum, 0,
+ eloc->logicalBlockNum, count,
partmap->s_partition_len);
goto error_return;
}
iinfo = UDF_I(table);
- udf_add_free_space(sb, sbi->s_partition, count);
+ udf_update_free_space(sb, sbi->s_partition, count);
- start = bloc->logicalBlockNum + offset;
- end = bloc->logicalBlockNum + offset + count - 1;
+ start = eloc->logicalBlockNum + offset;
+ end = eloc->logicalBlockNum + offset + count - 1;
epos.offset = oepos.offset = sizeof(struct unallocSpaceEntry);
elen = 0;
@@ -381,12 +456,12 @@ static void udf_table_free_blocks(struct super_block *sb,
epos.bh = oepos.bh = NULL;
while (count &&
- (etype = udf_next_aext(table, &epos, &eloc, &elen, 1)) != -1) {
- if (((eloc.logicalBlockNum +
+ (etype = udf_next_aext(table, &epos, &bloc, &elen, 1)) != -1) {
+ if (((bloc.logicalBlockNum +
(elen >> sb->s_blocksize_bits)) == start)) {
if ((0x3FFFFFFF - elen) <
(count << sb->s_blocksize_bits)) {
- uint32_t tmp = ((0x3FFFFFFF - elen) >>
+ int tmp = ((0x3FFFFFFF - elen) >>
sb->s_blocksize_bits);
count -= tmp;
start += tmp;
@@ -399,26 +474,26 @@ static void udf_table_free_blocks(struct super_block *sb,
start += count;
count = 0;
}
- udf_write_aext(table, &oepos, &eloc, elen, 1);
- } else if (eloc.logicalBlockNum == (end + 1)) {
+ udf_write_aext(table, &oepos, &bloc, elen, 1);
+ } else if (bloc.logicalBlockNum == (end + 1)) {
if ((0x3FFFFFFF - elen) <
(count << sb->s_blocksize_bits)) {
- uint32_t tmp = ((0x3FFFFFFF - elen) >>
+ int tmp = ((0x3FFFFFFF - elen) >>
sb->s_blocksize_bits);
count -= tmp;
end -= tmp;
- eloc.logicalBlockNum -= tmp;
+ bloc.logicalBlockNum -= tmp;
elen = (etype << 30) |
(0x40000000 - sb->s_blocksize);
} else {
- eloc.logicalBlockNum = start;
+ bloc.logicalBlockNum = start;
elen = (etype << 30) |
(elen +
(count << sb->s_blocksize_bits));
end -= count;
count = 0;
}
- udf_write_aext(table, &oepos, &eloc, elen, 1);
+ udf_write_aext(table, &oepos, &bloc, elen, 1);
}
if (epos.bh != oepos.bh) {
@@ -448,7 +523,7 @@ static void udf_table_free_blocks(struct super_block *sb,
int adsize;
- eloc.logicalBlockNum = start;
+ bloc.logicalBlockNum = start;
elen = EXT_RECORDED_ALLOCATED |
(count << sb->s_blocksize_bits);
@@ -464,16 +539,16 @@ static void udf_table_free_blocks(struct super_block *sb,
if (epos.offset + (2 * adsize) > sb->s_blocksize) {
/* Steal a block from the extent being free'd */
- udf_setup_indirect_aext(table, eloc.logicalBlockNum,
+ udf_setup_indirect_aext(table, bloc.logicalBlockNum,
&epos);
- eloc.logicalBlockNum++;
+ bloc.logicalBlockNum++;
elen -= sb->s_blocksize;
}
/* It's possible that stealing the block emptied the extent */
if (elen)
- __udf_add_aext(table, &epos, &eloc, elen, 1);
+ __udf_add_aext(table, &epos, &bloc, elen, 1);
}
brelse(epos.bh);
@@ -484,16 +559,28 @@ static void udf_table_free_blocks(struct super_block *sb,
return;
}
+/**
+ * udf_table_prealloc_blocks() - Allocate partition blocks
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space table
+ * @partition: UDF partition to use
+ * @first_block: Start of a free, or for write once media unrecorded, extent
+ * @count: Maximum number of blocks to try and allocate
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ *
+ * Return: Number of blocks allocated on success or 0 on failure
+ */
static int udf_table_prealloc_blocks(struct super_block *sb,
- struct inode *table, uint16_t partition,
- uint32_t first_block, uint32_t block_count)
+ struct inode *table, int partition,
+ sector_t first_block, int count)
{
struct udf_sb_info *sbi = UDF_SB(sb);
int alloc_count = 0;
- uint32_t elen, adsize;
+ int elen, adsize;
struct kernel_lb_addr eloc;
struct extent_position epos;
- int8_t etype = -1;
+ int etype = -1;
struct udf_inode_info *iinfo;
if (first_block >= sbi->s_partmaps[partition].s_partition_len)
@@ -515,8 +602,8 @@ static int udf_table_prealloc_blocks(struct super_block *sb,
while (first_block != eloc.logicalBlockNum &&
(etype = udf_next_aext(table, &epos, &eloc, &elen, 1)) != -1) {
- udf_debug("eloc=%d, elen=%d, first_block=%d\n",
- eloc.logicalBlockNum, elen, first_block);
+ udf_debug("eloc=%d, elen=%d, first_block=%llu\n",
+ eloc.logicalBlockNum, elen, (unsigned long long)first_block);
; /* empty loop body */
}
@@ -524,8 +611,8 @@ static int udf_table_prealloc_blocks(struct super_block *sb,
epos.offset -= adsize;
alloc_count = (elen >> sb->s_blocksize_bits);
- if (alloc_count > block_count) {
- alloc_count = block_count;
+ if (alloc_count > count) {
+ alloc_count = count;
eloc.logicalBlockNum += alloc_count;
elen -= (alloc_count << sb->s_blocksize_bits);
udf_write_aext(table, &epos, &eloc,
@@ -540,22 +627,32 @@ static int udf_table_prealloc_blocks(struct super_block *sb,
brelse(epos.bh);
if (alloc_count)
- udf_add_free_space(sb, partition, -alloc_count);
+ udf_update_free_space(sb, partition, -alloc_count);
mutex_unlock(&sbi->s_alloc_mutex);
return alloc_count;
}
-static int udf_table_new_block(struct super_block *sb,
- struct inode *table, uint16_t partition,
- uint32_t goal, int *err)
+/**
+ * udf_table_new_block() - Allocate a partition block close to goal
+ * @sb: Super block of UDF filesystem mount
+ * @bitmap: Space table
+ * @partition: UDF partition to use
+ * @goal: The block on the partition to which proximity is desired
+ * @err: If not successful this is the error that caused the failure
+ *
+ * Return: Partition block number on success or 0 and set *err on failure
+ */
+static sector_t udf_table_new_block(struct super_block *sb,
+ struct inode *table, int partition,
+ sector_t goal, int *err)
{
struct udf_sb_info *sbi = UDF_SB(sb);
- uint32_t spread = 0xFFFFFFFF, nspread = 0xFFFFFFFF;
- uint32_t newblock = 0, adsize;
- uint32_t elen, goal_elen = 0;
+ unsigned int spread = 0xFFFFFFFF, nspread = 0xFFFFFFFF;
+ sector_t newblock = 0;
+ int elen, adsize, goal_elen = 0;
struct kernel_lb_addr eloc, uninitialized_var(goal_eloc);
struct extent_position epos, goal_epos;
- int8_t etype;
+ int etype;
struct udf_inode_info *iinfo = UDF_I(table);
*err = -ENOSPC;
@@ -572,8 +669,8 @@ static int udf_table_new_block(struct super_block *sb,
goal = 0;
/* We search for the closest matching block to goal. If we find
- a exact hit, we stop. Otherwise we keep going till we run out
- of extents. We store the buffer_head, bloc, and extoffset
+ an exact hit, we stop. Otherwise we keep going till we run out
+ of extents. We store the buffer_head, eloc, and offset
of the current closest match and use that when we are done.
*/
epos.offset = sizeof(struct unallocSpaceEntry);
@@ -630,44 +727,68 @@ static int udf_table_new_block(struct super_block *sb,
udf_delete_aext(table, goal_epos, goal_eloc, goal_elen);
brelse(goal_epos.bh);
- udf_add_free_space(sb, partition, -1);
+ udf_update_free_space(sb, partition, -1);
mutex_unlock(&sbi->s_alloc_mutex);
*err = 0;
return newblock;
}
+/**
+ * udf_free_blocks() - Free extent blocks
+ * @sb: Super block of UDF filesystem mount
+ * @inode: If not NULL decrease the byte count allocated to the inode
+ * @eloc: Extent location/length descriptor
+ * @offset: Block offset from the start of the extent
+ * @count: Number of blocks to free
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ * Use the media appropriate bitmap/table routine for the sb and partition
+ */
void udf_free_blocks(struct super_block *sb, struct inode *inode,
- struct kernel_lb_addr *bloc, uint32_t offset,
- uint32_t count)
+ struct kernel_lb_addr *eloc, sector_t offset,
+ int count)
{
- uint16_t partition = bloc->partitionReferenceNum;
+ int partition = eloc->partitionReferenceNum;
struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_BITMAP) {
udf_bitmap_free_blocks(sb, map->s_uspace.s_bitmap,
- bloc, offset, count);
+ eloc, offset, count);
} else if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_TABLE) {
udf_table_free_blocks(sb, map->s_uspace.s_table,
- bloc, offset, count);
+ eloc, offset, count);
} else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP) {
udf_bitmap_free_blocks(sb, map->s_fspace.s_bitmap,
- bloc, offset, count);
+ eloc, offset, count);
} else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE) {
udf_table_free_blocks(sb, map->s_fspace.s_table,
- bloc, offset, count);
+ eloc, offset, count);
}
if (inode) {
inode_sub_bytes(inode,
- ((sector_t)count) << sb->s_blocksize_bits);
+ ((loff_t)count) << sb->s_blocksize_bits);
}
}
+/**
+ * udf_prealloc_blocks() - Allocate partition blocks and reserve them
+ * @sb: Super block of UDF filesystem mount
+ * @inode: If not NULL increase the byte count allocated to the inode
+ * @partition: UDF partition to use
+ * @first_block: Start of a free, or for write once media unrecorded, extent
+ * @count: Maximum number of blocks to try and allocate
+ *
+ * All the blocks are in the same extent which limits the maximum block count
+ * Use the media appropriate bitmap/table routine for the sb and partition
+ *
+ * Return: Number of blocks allocated on success or 0 on failure
+ */
inline int udf_prealloc_blocks(struct super_block *sb,
struct inode *inode,
- uint16_t partition, uint32_t first_block,
- uint32_t block_count)
+ int partition, sector_t first_block,
+ int count)
{
struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
int allocated;
@@ -676,22 +797,22 @@ inline int udf_prealloc_blocks(struct super_block *sb,
allocated = udf_bitmap_prealloc_blocks(sb,
map->s_uspace.s_bitmap,
partition, first_block,
- block_count);
+ count);
else if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_TABLE)
allocated = udf_table_prealloc_blocks(sb,
map->s_uspace.s_table,
partition, first_block,
- block_count);
+ count);
else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP)
allocated = udf_bitmap_prealloc_blocks(sb,
map->s_fspace.s_bitmap,
partition, first_block,
- block_count);
+ count);
else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE)
allocated = udf_table_prealloc_blocks(sb,
map->s_fspace.s_table,
partition, first_block,
- block_count);
+ count);
else
return 0;
@@ -700,34 +821,47 @@ inline int udf_prealloc_blocks(struct super_block *sb,
return allocated;
}
-inline int udf_new_block(struct super_block *sb,
+/**
+ * udf_new_block() - Allocate a partition block close to goal
+ * @sb: Super block of UDF filesystem mount
+ * @inode: If not NULL increase the byte count allocated to the inode
+ * @partition: UDF partition to use
+ * @goal: The block on the partition to which proximity is desired
+ * @err: If not successful this is the error that caused the failure
+ *
+ * Use the media appropriate bitmap/table routine for the sb and partition
+ * Reduce latency for things like directory expansion - Maybe "near" instead of "new"?
+ *
+ * Return: Partition block number on success or 0 and set *err on failure
+ */
+inline sector_t udf_new_block(struct super_block *sb,
struct inode *inode,
- uint16_t partition, uint32_t goal, int *err)
+ int partition, sector_t goal, int *err)
{
struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
- int block;
+ sector_t newblock;
if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_BITMAP)
- block = udf_bitmap_new_block(sb,
+ newblock = udf_bitmap_new_block(sb,
map->s_uspace.s_bitmap,
partition, goal, err);
else if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_TABLE)
- block = udf_table_new_block(sb,
+ newblock = udf_table_new_block(sb,
map->s_uspace.s_table,
partition, goal, err);
else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP)
- block = udf_bitmap_new_block(sb,
+ newblock = udf_bitmap_new_block(sb,
map->s_fspace.s_bitmap,
partition, goal, err);
else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE)
- block = udf_table_new_block(sb,
+ newblock = udf_table_new_block(sb,
map->s_fspace.s_table,
partition, goal, err);
else {
*err = -EIO;
return 0;
}
- if (inode && block)
+ if (inode && newblock)
inode_add_bytes(inode, sb->s_blocksize);
- return block;
+ return newblock;
}
--
2.9.2.277.g2949358
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V3 1/1] fs/udf RFC sanitize types, use consistent naming and add kernel doc comments
2017-01-14 17:37 [PATCH V3 1/1] fs/udf RFC sanitize types, use consistent naming and add kernel doc comments Steve Kenton
@ 2017-01-16 14:54 ` Jan Kara
0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2017-01-16 14:54 UTC (permalink / raw)
To: Steve Kenton; +Cc: linux-fsdevel, jack
On Sat 14-01-17 11:37:06, Steve Kenton wrote:
> +/*
> + * Unallocated/freespace bitmap implementation routines
> + */
> +
> +/**
> + * udf_bitmap_free_blocks() - Free extent blocks
> + * @sb: Super block of UDF filesystem mount
> + * @bitmap: Space bitmap
> + * @eloc: Extent location/length descriptor
'eloc' is only extent start, not length... It appears several times in the
comments.
> +/*
> + * Unallocated/freespace table implementation routines
> + */
> +
> +/**
> + * udf_table_free_blocks() - Free extent blocks
> + * @sb: Super block of UDF filesystem mount
> + * @bitmap: Space table
> + * @eloc: Extent location/length descriptor
> + * @offset: Block offset from the start of the extent
> + * @count: Number of blocks to free
> + *
> + * All the blocks are in the same extent which limits the maximum block count
> + */
> static void udf_table_free_blocks(struct super_block *sb,
> struct inode *table,
> - struct kernel_lb_addr *bloc,
> - uint32_t offset,
> - uint32_t count)
> + struct kernel_lb_addr *eloc,
> + sector_t offset,
> + int count)
> {
> struct udf_sb_info *sbi = UDF_SB(sb);
> struct udf_part_map *partmap;
> - uint32_t start, end;
> - uint32_t elen;
> - struct kernel_lb_addr eloc;
> + sector_t start, end;
> + int elen;
> + struct kernel_lb_addr bloc;
> struct extent_position oepos, epos;
> - int8_t etype;
> + int etype;
> struct udf_inode_info *iinfo;
In this function I find the changes of names confusing. You changed 'bloc'
parameter to 'eloc' which makes sense because it is a beginning of an
extent in which we are freeing. Fine. However then you had to rename local
variable 'eloc' and you renamed it to 'bloc'. This makes is difficult to
catch possible forgotten conversion in this function and also the name
'bloc' does not make sense (it is beginning of an extent in the free table)
and neither it is consistent with matching 'epos', 'etype', and 'elen'
variables. So either leave the argument and local variable names as is, or
rename them so that corresponding variables stay consistenly - something like
'teloc', 'telen', 'tetype', 'tepos'... for 'table extent location'.
Otherwise the patch looks good to me. Thanks!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-01-16 14:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-14 17:37 [PATCH V3 1/1] fs/udf RFC sanitize types, use consistent naming and add kernel doc comments Steve Kenton
2017-01-16 14:54 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).