On Fri, Nov 16, 2018 at 03:03:14PM +0100, Christoph Hellwig wrote:
> It seems like bi_phys_segments is still around of this series.
> Shouldn't it be superflous now?
Even though multi-page bvec is supported, the segment number doesn't
equal to the actual bvec count yet, for example, one bvec may b
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
btrfs_can_relocate returns 0 when it concludes the given chunk can be
relocated and -1 otherwise. Since this function is used as a predicated
and it return a binary value it makes no sense to have it's return
value as an int so change it to bool.
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
lock_delalloc_pages should only return 2 values - 0 in case of success
and -EAGAIN if the range of pages to be locked should be shrunk due to
some of gone. Manual inspections confirms that this is
indeed the case since __process_pages_contig is wh
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
All callers of this function pass BTRFS_MAX_EXTENT_SIZE (128M) so let's
reduce the argument count and make that a local variable. No functional
changes.
Signed-off-by: Nikolay Borisov
Reviewed-by: Anand Jain
Thanks, Anand
---
fs/btrfs/
On Thu, Nov 15, 2018 at 10:39 AM Juan Alberto Cirez
wrote:
>
> Is BTRFS mature enough to be deployed on a production system to underpin
> the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)?
>
> Based on our limited experience with BTRFS (1+ year) under the above
> scenario the a
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
It's unnecessary to check map->stripes[i].dev for NULL given its value
is already set and dereferenced above the the check. No functional changes.
Signed-off-by: Nikolay Borisov
Reviewed-by: Anand Jain
Thanks, Anand
---
fs/btrfs/volumes.
On Fri, Nov 16, 2018 at 08:06:36PM +0800, Anand Jain wrote:
>
>
> On 11/15/2018 11:35 PM, David Sterba wrote:
> > On Sun, Nov 11, 2018 at 10:22:22PM +0800, Anand Jain wrote:
> >> When we successfully cancel the replace its scrub returns -ECANCELED,
> >> which then passed to btrfs_dev_replace_fini
On 16.11.18 г. 18:17 ч., Stephan Olbrich wrote:
> Hi,
>
> a few days ago my root file system (simple btrfs on a SSD, no RAID or
> anything) suddenly became read only.
> Looking at dmsg, I found this:
>
> [ 19.285020] BTRFS error (device sda2): bad tree block start, want
> 705757184 have 8
On Thu, Nov 15, 2018 at 04:53:06PM +0800, Ming Lei wrote:
> QUEUE_FLAG_NO_SG_MERGE has been killed, so kill BLK_MQ_F_SG_MERGE too.
>
> Cc: Dave Chinner
> Cc: Kent Overstreet
> Cc: Mike Snitzer
> Cc: dm-de...@redhat.com
> Cc: Alexander Viro
> Cc: linux-fsde...@vger.kernel.org
> Cc: Shaohua Li
On Fri, Nov 16, 2018 at 02:59:22PM +0100, Christoph Hellwig wrote:
> On Thu, Nov 15, 2018 at 06:18:11PM -0800, Omar Sandoval wrote:
> > This commit message wasn't very clear. Is it the case that
> > QUEUE_FLAG_NO_SG_MERGE is no longer set by any drivers?
>
> I think he wants to say that not doing
Hi,
a few days ago my root file system (simple btrfs on a SSD, no RAID or anything)
suddenly became read only.
Looking at dmsg, I found this:
[ 19.285020] BTRFS error (device sda2): bad tree block start, want 705757184
have 82362368
[ 19.285042] BTRFS: error (device sda2) in __btrfs_free_e
On Fri, Nov 16, 2018 at 10:19:56AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 15, 2018 at 02:18:47PM -0800, Omar Sandoval wrote:
> > My only reason to prefer unsigned int is consistency. unsigned int is
> > much more common in the kernel:
> >
> > $ ag --cc -s 'unsigned\s+int' | wc -l
> > 129632
On 16.11.18 г. 17:18 ч., David Sterba wrote:
> On Fri, Oct 26, 2018 at 02:43:16PM +0300, Nikolay Borisov wrote:
>> While investigating the balance hang I came across various inconsistencies
>> in
>> the source. This series aims to fix those.
>>
>> The first patch is (I believe) a fix to a lon
On 16.11.18 г. 15:04 ч., fdman...@kernel.org wrote:
> From: Filipe Manana
>
> We can have a lot freed extents during the life span of transaction, so
> the red black tree that keeps track of the ranges of each freed extent
> (fs_info->freed_extents[]) can get quite big. When finishing a transa
On Fri, Oct 26, 2018 at 02:43:16PM +0300, Nikolay Borisov wrote:
> While investigating the balance hang I came across various inconsistencies in
> the source. This series aims to fix those.
>
> The first patch is (I believe) a fix to a longstanding bug that could cause
> balance to fail due to
It seems like bi_phys_segments is still around of this series.
Shouldn't it be superflous now?
On Thu, Nov 15, 2018 at 04:53:06PM +0800, Ming Lei wrote:
> QUEUE_FLAG_NO_SG_MERGE has been killed, so kill BLK_MQ_F_SG_MERGE too.
Looks fine,
Reviewed-by: Christoph Hellwig
On Thu, Nov 15, 2018 at 06:18:11PM -0800, Omar Sandoval wrote:
> This commit message wasn't very clear. Is it the case that
> QUEUE_FLAG_NO_SG_MERGE is no longer set by any drivers?
I think he wants to say that not doing S/G merging is rather pointless
with the current setup of the I/O path, as it
On Thu, Nov 15, 2018 at 04:53:05PM +0800, Ming Lei wrote:
> Since bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting"),
> physical segment number is mainly figured out in blk_queue_split() for
> fast path, and the flag of BIO_SEG_VALID is set there too.
>
> Now only blk_recount_s
On Thu, Nov 15, 2018 at 04:53:04PM +0800, Ming Lei wrote:
> It is wrong to use bio->bi_vcnt to figure out how many segments
> there are in the bio even though CLONED flag isn't set on this bio,
> because this bio may be splitted or advanced.
>
> So always use bio_segments() in blk_recount_segments
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 5040e9a2eb09..277921ad42e7 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -34,15 +34,7 @@
> #define BIO_BUG_ON
> #endif
>
> -#ifdef CONFIG_THP_SWAP
> -#if HPAGE_PMD_NR > 256
> -#define BIO_MAX_PAGES
> -
> - if (page == bv->bv_page && off == bv->bv_offset + bv->bv_len) {
> - bv->bv_len += len;
> - bio->bi_iter.bi_size += len;
> - return true;
> - }
> + struct request_queue *q = NULL;
> +
> +
I'd much rather have __bio_try_merge_page only do merges in
the same page, and have a new __bio_try_merge_segment that does
multi-page merges. This will keep the accounting a lot simpler.
> - bio_for_each_segment_all(bv, bio, i) {
> + for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) {
This really needs a comment. Otherwise it looks fine to me.
On Thu, Nov 15, 2018 at 04:52:56PM +0800, Ming Lei wrote:
> There are still cases in which we need to use bio_bvecs() for get the
> number of multi-page segment, so introduce it.
The only user in your final tree seems to be the loop driver, and
even that one only uses the helper for read/write bio
On 11/16/2018 03:51 AM, Nikolay Borisov wrote:
On 15.11.18 г. 20:39 ч., Juan Alberto Cirez wrote:
Is BTRFS mature enough to be deployed on a production system to underpin
the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)?
Based on our limited experience with BTRFS (1+ y
On Thu, Nov 15, 2018 at 04:52:55PM +0800, Ming Lei wrote:
> BTRFS is the only user of this helper, so move this helper into
> BTRFS, and implement it via bio_for_each_segment_all(), since
> bio->bi_vcnt may not equal to number of pages after multipage bvec
> is enabled.
btrfs only uses the value t
On Thu, Nov 15, 2018 at 04:52:54PM +0800, Ming Lei wrote:
> index 2955a4ea2fa8..161e14b8b180 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -400,8 +400,11 @@ blk_status_t btrfs_submit_compressed_write(struct inode
> *inode, u64 start,
> static u64 bio_end_offset(struct
Looks fine,
Reviewed-by: Christoph Hellwig
> + unsigned last_page = total / PAGE_SIZE;
> +
> + if (last_page * PAGE_SIZE == total)
Not sure it matters much with modern compilers, but generally
we shit by PAGE_SHIFT instead of multiplying with or dividing
by PAGE_SIZE.
> + if (!*sg)
> + return sglist;
> + else {
No need for an else after an early return.
> +static inline void __bio_advance_iter(struct bio *bio, struct bvec_iter
> *iter,
> + unsigned bytes, bool mp)
I think these magic 'bool np' arguments and wrappers over wrapper
don't help anyone to actually understand the code. I'd vote for
removing as many wr
> -#define bvec_iter_page(bvec, iter) \
> +#define mp_bvec_iter_page(bvec, iter)\
> (__bvec_iter_bvec((bvec), (iter))->bv_page)
>
> -#define bvec_iter_len(bvec, iter)\
> +#define mp_bvec_iter_len(bvec, ite
From: Filipe Manana
We can have a lot freed extents during the life span of transaction, so
the red black tree that keeps track of the ranges of each freed extent
(fs_info->freed_extents[]) can get quite big. When finishing a transaction
commit we find each range, process it (discard the extents,
On 2018-11-15 13:39, Juan Alberto Cirez wrote:
Is BTRFS mature enough to be deployed on a production system to underpin
the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)?
For NVR, I'd say no. BTRFS does pretty horribly with append-only
workloads, even if they are WORM style.
On 11/15/2018 11:35 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:22PM +0800, Anand Jain wrote:
When we successfully cancel the replace its scrub returns -ECANCELED,
which then passed to btrfs_dev_replace_finishing(), it cleans up based
on the scrub returned status and propagates the
From: Filipe Manana
Commit d7396f07358a ("Btrfs: optimize key searches in btrfs_search_slot"),
dated from August 2013, introduced an optimization to search for keys in a
node/leaf to both btrfs_search_slot() and btrfs_search_old_slot(). For the
later, it ended up being reverted in commit d4b4087c
On 11/16/2018 06:29 PM, Anand Jain wrote:
On 11/15/2018 11:31 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:23PM +0800, Anand Jain wrote:
As of now only user requested replace cancel can cancel the
replace-scrub
so no need to log error for it.
This has probably some user visible
On 11/15/2018 11:31 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:23PM +0800, Anand Jain wrote:
As of now only user requested replace cancel can cancel the replace-scrub
so no need to log error for it.
This has probably some user visible effect or threre are steps to
reproduce the m
On 2018/11/16 17:19, Christoph Hellwig wrote:
> On Thu, Nov 15, 2018 at 02:18:47PM -0800, Omar Sandoval wrote:
>> My only reason to prefer unsigned int is consistency. unsigned int is
>> much more common in the kernel:
>>
>> $ ag --cc -s 'unsigned\s+int' | wc -l
>> 129632
>> $ ag --cc -s 'unsigne
On 11/15/2018 11:41 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:15PM +0800, Anand Jain wrote:
v1->v2:
2/9: Drop writeback required
3/9: Drop writeback required
7/9: Use the condition within the WARN_ON()
6/9: Use the condition within the ASSERT()
Replace-start and repla
On Thu, Nov 15, 2018 at 02:18:47PM -0800, Omar Sandoval wrote:
> My only reason to prefer unsigned int is consistency. unsigned int is
> much more common in the kernel:
>
> $ ag --cc -s 'unsigned\s+int' | wc -l
> 129632
> $ ag --cc -s 'unsigned\s+(?!char|short|int|long)' | wc -l
> 22435
>
> check
On 16.11.18 г. 10:22 ч., Qu Wenruo wrote:
> GCC 8.2.1 will report the following warning with "make W=1":
>
> ctree.c: In function 'btrfs_next_sibling_tree_block':
> ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
> path->slots[level
GCC 8.2.1 will report the following warning with "make W=1":
ctree.c: In function 'btrfs_next_sibling_tree_block':
ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function
[-Wmaybe-uninitialized]
path->slots[level] = slot;
~~~^~
The culprit is t
On 2018/11/16 下午4:13, Nikolay Borisov wrote:
>
>
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> The only location is the following code:
>>
>> int level = path->lowest_level + 1;
>> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>> while(level < BTRFS_MAX_LEVEL) {
>>
On 16.11.18 г. 10:04 ч., Qu Wenruo wrote:
> The following missing prototypes will be fixed:
> 1) btrfs.c::handle_special_globals()
> 2) check/mode-lowmem.c::repair_ternary_lowmem()
> 3) extent-tree.c::btrfs_search_overlap_extent()
> Above 3 can be fixed by making them static
>
> 4) util
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only location is the following code:
>
> int level = path->lowest_level + 1;
> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> while(level < BTRFS_MAX_LEVEL) {
> slot = path->slots[level] + 1;
> ..
On 2018/11/16 下午4:04, Nikolay Borisov wrote:
>
>
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> Although most fallthrough case is pretty obvious, we still need to teach
>> the dumb compiler that it's an explicit fallthrough.
>>
>> Also reformat the code to use common indent.
>>
>> Signed-off-by
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only hit is the following code:
>
> tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
>
> if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
> || tlv_len > BTRFS_SEND_BUF_SIZE) {
> error("in
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Add __attribute__ ((format (printf, 4, 0))) to fix the vprintf calling
> function.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> string-table.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/string-table.c b/string-t
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Although most fallthrough case is pretty obvious, we still need to teach
> the dumb compiler that it's an explicit fallthrough.
>
> Also reformat the code to use common indent.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
Is this attr
The following missing prototypes will be fixed:
1) btrfs.c::handle_special_globals()
2) check/mode-lowmem.c::repair_ternary_lowmem()
3) extent-tree.c::btrfs_search_overlap_extent()
Above 3 can be fixed by making them static
4) utils.c::btrfs_check_nodesize()
Fixed by moving it to fsfea
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> messages.h:49:24: warning: suggest braces around empty body in an 'if'
> statement [-Wempty-body]
> PRINT_TRACE_ON_ERROR;\
>
> Just extra braces would solve the problem.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> m
Under most case, we are just using 'int' for 'unsigned int', and doesn't
care about the sign.
The Wsign-compare is causing tons of false alerts.
Suppressing it would make W=1 less noisy.
Signed-off-by: Qu Wenruo
---
changelog:
v1.1:
Use cc-disable-warning to provide much better compatibility
54 matches
Mail list logo
