| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20121011095044.6c88e3d4@notabene.brown>
Date: Thu, 11 Oct 2012 09:50:44 +1100
From: NeilBrown <neilb@...e.de>
To: 김재극 <jaegeuk.kim@...sung.com>
Cc: viro@...iv.linux.org.uk, 'Theodore Ts'o' <tytso@....edu>,
gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
chur.lee@...sung.com, cm224.lee@...sung.com,
jooyoung.hwang@...sung.com
Subject: Re: [PATCH 03/16] f2fs: add superblock and major in-memory
structures
On Fri, 05 Oct 2012 20:57:46 +0900 김재극 <jaegeuk.kim@...sung.com> wrote:
> +static inline unsigned int curseg_segno(struct f2fs_sb_info *sbi,
> + int type)
> +{
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> + unsigned int segno;
> + mutex_lock(&curseg->curseg_mutex);
> + segno = curseg->segno;
> + mutex_unlock(&curseg->curseg_mutex);
> + return segno;
> +}
> +
> +static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi,
> + int type)
> +{
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> + unsigned char a_type;
> + mutex_lock(&curseg->curseg_mutex);
> + a_type = curseg->alloc_type;
> + mutex_unlock(&curseg->curseg_mutex);
> + return a_type;
> +}
> +
> +static inline unsigned short curseg_blkoff(struct f2fs_sb_info *sbi, int type)
> +{
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> + unsigned short blkoff;
> + mutex_lock(&curseg->curseg_mutex);
> + blkoff = curseg->next_blkoff;
> + mutex_unlock(&curseg->curseg_mutex);
> + return blkoff;
> +}
Taking a mutex just to extract a small number from a structure is pointless.
alloc_type, next_blkoff and segno are char, short, and int. All of these can
be read atomically, so a lock gains you nothing.
In checkpoint.c we have
for (i = 0; i < 3; i++) {
ckpt->cur_node_segno[i] =
cpu_to_le32(curseg_segno(sbi, i + CURSEG_HOT_NODE));
ckpt->cur_node_blkoff[i] =
cpu_to_le16(curseg_blkoff(sbi, i + CURSEG_HOT_NODE));
nat_upd_blkoff[i] = NM_I(sbi)->nat_upd_blkoff[i];
ckpt->nat_upd_blkoff[i] = cpu_to_le16(nat_upd_blkoff[i]);
ckpt->alloc_type[i + CURSEG_HOT_NODE] =
curseg_alloc_type(sbi, i + CURSEG_HOT_NODE);
}
which will take and drop that same lock 3 times in quick succession, and then
do it again for 3 other locks (And there is another loop which does it for
the other 3 cursegs).
If you do need some locking here, I think you need to take the lock once per
loop iteration so the 3 values are consistent, not once for each value.
Regards,
NeilBrown
> +
> +static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno)
> +{
> + unsigned int end_segno = SM_I(sbi)->segment_count - 1;
> + BUG_ON(segno > end_segno);
> +}
> +
> +/*
> + * This function is used for only debugging.
> + * NOTE: In future, we have to remove this function.
> + */
> +static inline void verify_block_addr(struct f2fs_sb_info *sbi, block_t blk_addr)
> +{
> + struct f2fs_sm_info *sm_info = SM_I(sbi);
> + block_t total_blks = sm_info->segment_count << sbi->log_blocks_per_seg;
> + block_t start_addr = sm_info->seg0_blkaddr;
> + block_t end_addr = start_addr + total_blks - 1;
> + BUG_ON(blk_addr < start_addr);
> + BUG_ON(blk_addr > end_addr);
> +}
> +
> +/**
> + * Summary block is always treated as invalid block
> + */
> +static inline void check_block_count(struct f2fs_sb_info *sbi,
> + int segno, struct f2fs_sit_entry *raw_sit)
> +{
> + struct f2fs_sm_info *sm_info = SM_I(sbi);
> + unsigned int end_segno = sm_info->segment_count - 1;
> + int valid_blocks = 0;
> + int i;
> +
> + /* check segment usage */
> + BUG_ON(GET_SIT_VBLOCKS(raw_sit) > sbi->blocks_per_seg);
> +
> + /* check boundary of a given segment number */
> + BUG_ON(segno > end_segno);
> +
> + /* check bitmap with valid block count */
> + for (i = 0; i < sbi->blocks_per_seg; i++)
> + if (f2fs_test_bit(i, raw_sit->valid_map))
> + valid_blocks++;
> + BUG_ON(GET_SIT_VBLOCKS(raw_sit) != valid_blocks);
> +}
> +
> +static inline pgoff_t current_sit_addr(struct f2fs_sb_info *sbi,
> + unsigned int start)
> +{
> + struct sit_info *sit_i = SIT_I(sbi);
> + unsigned int offset = SIT_BLOCK_OFFSET(sit_i, start);
> + block_t blk_addr = sit_i->sit_base_addr + offset;
> +
> + check_seg_range(sbi, start);
> +
> + /* calculate sit block address */
> + if (f2fs_test_bit(offset, sit_i->sit_bitmap))
> + blk_addr += sit_i->sit_blocks;
> +
> + return blk_addr;
> +}
> +
> +static inline pgoff_t next_sit_addr(struct f2fs_sb_info *sbi,
> + pgoff_t block_addr)
> +{
> + struct sit_info *sit_i = SIT_I(sbi);
> + block_addr -= sit_i->sit_base_addr;
> + if (block_addr < sit_i->sit_blocks)
> + block_addr += sit_i->sit_blocks;
> + else
> + block_addr -= sit_i->sit_blocks;
> +
> + return block_addr + sit_i->sit_base_addr;
> +}
> +
> +static inline void set_to_next_sit(struct sit_info *sit_i, unsigned int start)
> +{
> + unsigned int block_off = SIT_BLOCK_OFFSET(sit_i, start);
> +
> + if (f2fs_test_bit(block_off, sit_i->sit_bitmap))
> + f2fs_clear_bit(block_off, sit_i->sit_bitmap);
> + else
> + f2fs_set_bit(block_off, sit_i->sit_bitmap);
> +}
> +
> +static inline uint64_t div64_64(uint64_t dividend, uint64_t divisor)
> +{
> + uint32_t d = divisor;
> +
> + if (divisor > 0xffffffffUll) {
> + unsigned int shift = fls(divisor >> 32);
> + d = divisor >> shift;
> + dividend >>= shift;
> + }
> +
> + if (dividend >> 32)
> + do_div(dividend, d);
> + else
> + dividend = (uint32_t) dividend / d;
> +
> + return dividend;
> +}
> +
> +static inline unsigned long long get_mtime(struct f2fs_sb_info *sbi)
> +{
> + struct sit_info *sit_i = SIT_I(sbi);
> + return sit_i->elapsed_time + CURRENT_TIME_SEC.tv_sec -
> + sit_i->mounted_time;
> +}
> +
> +static inline void set_summary(struct f2fs_summary *sum, nid_t nid,
> + unsigned int ofs_in_node, unsigned char version)
> +{
> + sum->nid = cpu_to_le32(nid);
> + sum->ofs_in_node = cpu_to_le16(ofs_in_node);
> + sum->version = version;
> +}
> +
> +static inline block_t start_sum_block(struct f2fs_sb_info *sbi)
> +{
> + return __start_cp_addr(sbi) +
> + le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> +}
> +
> +static inline block_t sum_blk_addr(struct f2fs_sb_info *sbi, int base, int type)
> +{
> + return __start_cp_addr(sbi) +
> + le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_total_block_count)
> + - (base + 1) + type;
> +}
Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)
Powered by blists - more mailing lists