-
Notifications
You must be signed in to change notification settings - Fork 3
Wip strong claim #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: upstream-1202
Are you sure you want to change the base?
Conversation
A strong claim is almost equivalent to claim, but deep copies volatile buffers to new raw_char buffers. A volatile buffer is one which cannot be held/claimed for long periods due to external dependencies (e.g., Accelio message buffers are volatile). Strong claim is now the unmarked case, so is performed automatically in the buffer::list copy constructor. Low-level operations should allow explicit volatile sharing, so copying a buffer::list:iterator or a buffer::ptr works as normal. For explicit sharing of volatile buffers, a new boolean is added to the public claim_* methods, and a buffer::list::share method is also added, to share an entire sequence. Signed-off-by: Vu Pham <[email protected]> Signed-off-by: Matt Benjamin <[email protected]> Signed-off-by: Matt Benjamin <[email protected]> Conflicts: src/include/buffer.h Signed-off-by: Matt Benjamin <[email protected]> Conflicts: src/common/buffer.cc
This change restores volatile sharing semantics in the Message decode path, and also in the OSD write path for FileStore/FileJournal. This can be verified with a breakpoint set at the clone/COW case in buffer::ptr::strong_claim (currently buffer.cc:690). Signed-off-by: Matt Benjamin <[email protected]> Signed-off-by: Matt Benjamin <[email protected]>
Signed-off-by: Matt Benjamin <[email protected]>
Signed-off-by: Matt Benjamin <[email protected]>
|
I have a quick look at accelio source codes. It looks like you want to let memory buffer which is used by ibv_rq can be append to bufferlist and recycle by accelio? |
|
Yeah, I'm not sure that I have got your idea. It looks like you can inherit "Raw" class which can add registered memory to bufferlist and a special destruction which can recycle memory when ptr->nref == 0. |
|
Thanks for reviewing. First, yes, the Xio changesets do derive a "raw" class with special destruction. We've had ones that just recycle the memory, and a stronger one does so via the related completion hook. You're right, that's the natural way to deal with zero-copy buffers in the current buffer API (yuyuyu101). The issue turned out to be, not getting zero-copy to work, but ensuring that zero-copy buffers were returned in a timely manner. Because buffer::ptrs have been shared by default, we don't know when (or even if) every ref will be returned. So what strong claim is attempting to do is make sharing !default -for a specific class of buffers- (ones with the volatile property). I wanted a way to ensure that, unless a code path would assuredly return volatile buffers "in a timely manner" (something that seemed difficult to ensure "by default"), it would not get zero-copy buffers (an Accelio pool resource, whether or not they are registered). Sage, will that have the same effect? (I'm genuinely not sure.) |
|
liewegas, Even though strong_claim_inplace() is called by default for claim() family and operator=; however, in buffer::ptr::strong_claim() we check if raw buffer volatile or not to deep copy. By the default the raw buffer is not volatile. We only set is_volatile() == true in case of Accelio's buffers. |
|
Okay, this makes sense. Perhaps renaming strong_claim_* to strong_claim_volatile_* would make this clearer. Also, instead of a bool strong=true arg, what about a bool force_weak_volatile=false? That would make it clearer that the exceptional case specific to volatile buffers? |
Reserve last char in array for '\0' to ensure termination of the string. Fix for: CID 1128383 (#1 of 1): Buffer not null terminated (BUFFER_SIZE_WARNING) buffer_size_warning: Calling strncpy with a maximum size argument of 1000 bytes on destination array secret of size 1000 bytes might leave the destination string unterminated. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for another case of: CID 717112 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable io_ctx going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <[email protected]>
Release completion as soon as no longer needed. Fix for: CID 1219593 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable completion going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1054876 (#1 of 1): Use after free (USE_AFTER_FREE) pass_freed_arg: Passing freed pointer bucket_obj as an argument to put_bucket_obj Signed-off-by: Danny Al-Gaaf <[email protected]>
Make sure k and m paramter are valid to prevent crash. Fix typo. Fix for the following CID and other possible invalid combinations of k/m parameter: CID 1219466 (#1 of 1): Division or modulo by zero (DIVIDE_BY_ZERO) divide_by_zero: In expression rand() % (k + m), modulo by expression k + m which may be zero has undefined behavior. Signed-off-by: Danny Al-Gaaf <[email protected]>
Check return value as done in all other places. Fix error messages to print correct function name getdir and not read_dir/readdir since the error isn't necessarily raised by read_dir(). Fix for: CID 1219463 (#1 of 1): Unchecked return value (CHECKED_RETURN) check_return: Calling getdir without checking return value (as is done elsewhere 4 out of 5 times). Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1054853 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking is_truncated suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Add vim line to file. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1128384 (#1 of 1): Ignoring number of bytes read (CHECKED_RETURN) check_return: fread(void * restrict, size_t, size_t, FILE * restrict) returns the number of bytes read, but it is ignored. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix memory leak caused by using std::string to hold result of strdup call returned from getObjName(). Fix for Coverity issues: CID 1221525 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Failing to save or free storage allocated by this->getObjName(soid, 0UL) leaks it. CID 1221526 (1-3 of 3): Resource leak (RESOURCE_LEAK) leaked_storage: Failing to save or free storage allocated by this->getObjName(soid, *) leaks it. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix indentation to the same as the original fsx.c . Fix for Coverity issue: CID 1219473 (#1-2 of 2): Nesting level does not match indentation (NESTING_INDENT_MISMATCH) uncle: This statement is indented to column 25, as if it were nested within the preceding parent statement, but it is not. Signed-off-by: Danny Al-Gaaf <[email protected]>
In the checks to build only_in_b up the wrong const_iterator x is build up. it should compare rhs->xattrs with xattrs entries and not twice rhs->xattrs. Fix for: CID 716957 (#1 of 1): Invalid iterator comparison (MISMATCHED_ITERATOR) mismatched_comparison: Comparing x from rhs->xattrs to this->xattrs.end() from this->xattrs. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1128391 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this statement goto done_err; Signed-off-by: Danny Al-Gaaf <[email protected]>
Wrong variable for return of rados_getxattrs_next() and rados_omap_get_next() used (len vs. val_len). Fix for: CID 1219464 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this expression key == NULL inside statement if (len == 0UL && key == NU... CID 1219465 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this expression key == NULL inside statement if (len == 0UL && key == NU... Signed-off-by: Danny Al-Gaaf <[email protected]>
Close socket if set_nonblock() fails before return. Fix for: CID 1249633 (#1 of 1): Resource leak (RESOURCE_LEAK) 7. leaked_handle: Handle variable s going out of scope leaks the handle. Signed-off-by: Danny Al-Gaaf <[email protected]>
Close socket if ::setsockopt() fails before return. Fix for: CID 1249632 (#1 of 1): Resource leak (RESOURCE_LEAK) 9. leaked_handle: Handle variable s going out of scope leaks the handle. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1251453 (#2 of 2): Resource leak (RESOURCE_LEAK) leaked_storage: Variable io_ctx going out of scope leaks the storage it points to. CID 1251454 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable h going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <[email protected]>
Remove useless check for invalid parameter for "mds remove_data_pool", we have already the poolid from the correct parameter 'poolname'. Fix for: CID 1251445 (#1 of 1): Unchecked return value (CHECKED_RETURN) check_return: Calling cmd_getval without checking return value (is done elsewhere 19 out of 22 times). Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 717142 (#1 of 1): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::buffer::end_of_buffer is thrown and never caught. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 717141 (#1-19): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::FailedAssertion is thrown and never caught. Signed-off-by: Danny Al-Gaaf <[email protected]>
CID 717157 (#1-2): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::FailedAssertion is thrown and never caught. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 717163 (#1 of 1): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char **) an exception of type ceph::buffer::end_of_buffer is thrown and never caught. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1297860 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS) 1. Condition this->mon_addr.count(n), taking true branch 2. negative_return_fn: Function this->get_rank(n) returns a negative number. [show details] 3. var_assign: Assigning: signed variable m = get_rank. 4. negative_returns: m is passed to a parameter that cannot be negative. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1188175 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. CID 1188174 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1254381 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member max_fd is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1249635 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member listen_sd is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 717237 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member field default_file_layout.fl_pg_pool is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_stripe_unit is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_stripe_count is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_object_size is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_cas_hash is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_object_stripe_unit is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_unused is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1026809 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member want_replica is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member want_xlocked is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member pool is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1026812 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member error is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1138594 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member last is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1160851 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member flushed_version is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1188164 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member bits is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1238901 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member ret1 is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ret2 is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ret3 is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1262114 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member map_epoch is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member acks_wanted is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1262115 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member map_epoch is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ack_type is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member result is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1297875 (#1 of 1): Arguments in wrong order (SWAPPED_ARGUMENTS) swapped_arguments: The positions of arguments in the call to do_lock_remove do not match the ordering of the parameters: lock_cookie is passed to client lock_client is passed to cookie Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1297874 (#1 of 1): Arguments in wrong order (SWAPPED_ARGUMENTS) swapped_arguments: The positions of arguments in the constructor for CompatSet do not match the ordering of the parameters: feature_incompat_base is passed to _ro_compat feature_ro_compat_base is passed to _incompat Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 717354 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member id is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member type is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member off is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member len is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member lg is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member completion is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1054871 (#1 of 2): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1019635 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1054872 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1188182 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member rcompletion is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1232607 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member m_dump_perf_counters is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_rbd is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_ioctx is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_latency_multiplier is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_readonly is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1274321 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member perr is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1274323 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member perr is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1297861 (#1 of 1): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) overflow_before_widen: Potentially overflowing expression this->layout.fl_stripe_count.operator __u32() * this->layout.fl_object_size.operator __u32() with type unsigned int (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type uint64_t (64 bits, unsigned). Signed-off-by: Danny Al-Gaaf <[email protected]>
Fix for: CID 1297885 (#1 of 2): Result is not floating-point (UNINTENDED_INTEGER_DIVISION) integer_division: Dividing integer expressions g_conf->mon_pool_quota_warn_threshold and 100, and then converting the integer quotient to type float. Any remainder, or fractional part of the quotient, is ignored. CID 1297885 (#2 of 2): Result is not floating-point (UNINTENDED_INTEGER_DIVISION) integer_division: Dividing integer expressions g_conf->mon_pool_quota_crit_threshold and 100, and then converting the integer quotient to type float. Any remainder, or fractional part of the quotient, is ignored. Signed-off-by: Danny Al-Gaaf <[email protected]>
Add assert to MonSessionMap::new_session(). Fix for: CID 1128408 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking s suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Signed-off-by: Danny Al-Gaaf <[email protected]>
CID 1322828 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 28. use_after_free: Using invalidated internal representation of local it. CID 1322827 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 25. use_after_free: Using invalidated internal representation of local it. CID 1322826 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 31. use_after_free: Using invalidated internal representation of local it. CID 1322825 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 31. use_after_free: Using invalidated internal representation of local it. Signed-off-by: Sage Weil <[email protected]>
CID 1322784 (#1 of 1): Uninitialized scalar variable (UNINIT) 2. uninit_use_in_call: Using uninitialized value coll.removal_seq when calling coll_t. [show details] Signed-off-by: Sage Weil <[email protected]>
CID 1322778 (#1 of 1): Pointer to local outside scope (RETURN_LOCAL) 1. escape_local_addr: Returning, through this->reqid, the address of stack variable _reqid. 2. return: Returning here. Signed-off-by: Sage Weil <[email protected]>
This is from "wip" - Sage Weil <[email protected]> Signed-off-by: Marcus Watts <[email protected]>
Proposed as a precursor to conditional XioMessenger changeseet.