Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 16:19, Philipp Reisner wrote: > Am Freitag, 9. November 2012, 15:50:24 schrieb Jens Axboe: >> On 2012-11-09 15:18, Jens Axboe wrote: >>> On 2012-11-09 14:33, Philipp Reisner wrote: Jens, here it is without the sysfs stuff >>> >>> Thanks, pulled into for-3.8/drivers >> >> I didn't say anything, but I've been fuming a bit the last few series of >> merge windows. You need to stop these insanely massive pull requests. >> I've been large since this is "just a driver", but it can't continue. We >> should have reached stability a long time ago. Your pull requests >> contain a shit load of items, are you guys paying per commit? Look at >> these: >> >> drbd: Request lookup code cleanup (1) >> drbd: Request lookup code cleanup (2) >> drbd: Request lookup code cleanup (3) >> drbd: Request lookup code cleanup (4) >> > > We are living there in the belief that we should break up big > changes in review able chunks Don't be silly. Of course things should be broken up. But they should be broken up in logical steps needed to convert from state X to Y. What does reqeust code cleanup (1) mean? Nothing. Lets take a look at the commit message: Move _ar_id_to_req() to drbd_receiver.c and mark it non-inline. Remove the leading underscores from _ar_id_to_req() and _ack_id_to_req(). Mark ar_hash_slot() inline. which is exactly the wrong way as well. A commit message should explain why a given change is done. The above describes how it's done. We can all read the code and see that. In other words, you need to school people on what a commit is, and what a series of commits is. >> or >> >> drbd: conn_send_cmd2(): Return 0 upon success and an error code >> otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code >> otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code >> otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code >> otherwise >> > > Function by function gets converted to the "return 0 upon success" call > semantics. > Do you prefer that all of that should be done in a single commit? For this case, yes indeed. Then it'd make more sense to describe why that change was made. For this particular one, it's fairly easy to guess why the change was needed. >> along with FIFTY or so more of these. WTF is this? >> >> drbd: Converted helper functions for drbd_send() to tconn >> drbd: Converted drbd_send() from mdev to tconn >> drbd: Converted drbd_send_fp() from mdev to tconn >> > > Should it instead be in a single commit? > >> >> I don't think I need to go on. So from now on, to get items into the >> kernel, what you will do is: >> >> - Stop doing insane commits like the above. It just doesn't make sense. >> >> - Send pull requests in a timely fashion. No more of this "lets collect >> ALL the things" then send it off. Collect small bug fixes, send those >> off. Develop some feature or make some changes, send that off. Etc. > > That works well for individual features, and we have been doing that > for the last two Years. > > But at this time we changed the object model. In the old code we had > a single kind of DRBD-in-kernel-object: a resource > Now we have two kinds: resources and volumes. > > 8.3: a resource had a single implicit volume > > 8.4: a resource might contain multiple volumes, each volume belongs to > a single resource. > > In the next ~12 month you will get only small features/updates etc... for > the 8.4 code base. If you had just structured it a bit better, then 8.3 -> 8.4 transition would look a lot less painful. As it stands right now, it's just a huge mess of little changes. But I'm happy that you think that the next 12 months will have only gradual changes for bug fixes and features. >> The fact that your initial pull request had to MASK all these commits >> should have rung big bells in your head. It's a clear sign of a huge >> problem in your development model. If you can't clean this up, then >> it's not going in. > > Fundamental changes in our object model require such huge change sets. > Jens, we will not stop where we are today. We plan to introduce a new > object: a connection. (The ability to mirror for one machine to *multiple* > receivers.) If you think I'm asking you to stop developing, then you are completely missing the message. I'm asking you to CHANGE how you develop, or at least present, these changes. Tons of little changes reak of "hey lets do this in this function. then lets also do this in that function". A bit more structure would go a long way. What I usually do is that I may develop from the first model, then refactor and structure everything to look a lot nicer. A big part of being able to review a patch series is understanding where it's going. Step 1 does this, step 2 does that, etc. If you have 500 changes of "change return code from void to err/0", well... It may be that you have nice structure underneath it and it's
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
Am Freitag, 9. November 2012, 15:50:24 schrieb Jens Axboe: > On 2012-11-09 15:18, Jens Axboe wrote: > > On 2012-11-09 14:33, Philipp Reisner wrote: > >> Jens, here it is without the sysfs stuff > > > > Thanks, pulled into for-3.8/drivers > > I didn't say anything, but I've been fuming a bit the last few series of > merge windows. You need to stop these insanely massive pull requests. > I've been large since this is "just a driver", but it can't continue. We > should have reached stability a long time ago. Your pull requests > contain a shit load of items, are you guys paying per commit? Look at > these: > > drbd: Request lookup code cleanup (1) > drbd: Request lookup code cleanup (2) > drbd: Request lookup code cleanup (3) > drbd: Request lookup code cleanup (4) > We are living there in the belief that we should break up big changes in review able chunks > or > > drbd: conn_send_cmd2(): Return 0 upon success and an error code > otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code > otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code > otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code > otherwise > Function by function gets converted to the "return 0 upon success" call semantics. Do you prefer that all of that should be done in a single commit? > along with FIFTY or so more of these. WTF is this? > > drbd: Converted helper functions for drbd_send() to tconn > drbd: Converted drbd_send() from mdev to tconn > drbd: Converted drbd_send_fp() from mdev to tconn > Should it instead be in a single commit? > > I don't think I need to go on. So from now on, to get items into the > kernel, what you will do is: > > - Stop doing insane commits like the above. It just doesn't make sense. > > - Send pull requests in a timely fashion. No more of this "lets collect > ALL the things" then send it off. Collect small bug fixes, send those > off. Develop some feature or make some changes, send that off. Etc. That works well for individual features, and we have been doing that for the last two Years. But at this time we changed the object model. In the old code we had a single kind of DRBD-in-kernel-object: a resource Now we have two kinds: resources and volumes. 8.3: a resource had a single implicit volume 8.4: a resource might contain multiple volumes, each volume belongs to a single resource. In the next ~12 month you will get only small features/updates etc... for the 8.4 code base. > The fact that your initial pull request had to MASK all these commits > should have rung big bells in your head. It's a clear sign of a huge > problem in your development model. If you can't clean this up, then > it's not going in. Fundamental changes in our object model require such huge change sets. Jens, we will not stop where we are today. We plan to introduce a new object: a connection. (The ability to mirror for one machine to *multiple* receivers.) Is it a better fit to introduce it then as a new driver? E.g. called it "drbd9". Should it use a new major number? Best, Phil -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 15:18, Jens Axboe wrote: > On 2012-11-09 14:33, Philipp Reisner wrote: >> Jens, here it is without the sysfs stuff > > Thanks, pulled into for-3.8/drivers I didn't say anything, but I've been fuming a bit the last few series of merge windows. You need to stop these insanely massive pull requests. I've been large since this is "just a driver", but it can't continue. We should have reached stability a long time ago. Your pull requests contain a shit load of items, are you guys paying per commit? Look at these: drbd: Request lookup code cleanup (1) drbd: Request lookup code cleanup (2) drbd: Request lookup code cleanup (3) drbd: Request lookup code cleanup (4) or drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise along with FIFTY or so more of these. WTF is this? drbd: Converted helper functions for drbd_send() to tconn drbd: Converted drbd_send() from mdev to tconn drbd: Converted drbd_send_fp() from mdev to tconn ... I don't think I need to go on. So from now on, to get items into the kernel, what you will do is: - Stop doing insane commits like the above. It just doesn't make sense. - Send pull requests in a timely fashion. No more of this "lets collect ALL the things" then send it off. Collect small bug fixes, send those off. Develop some feature or make some changes, send that off. Etc. The fact that your initial pull request had to MASK all these commits should have rung big bells in your head. It's a clear sign of a huge problem in your development model. If you can't clean this up, then it's not going in. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 14:33, Philipp Reisner wrote: > Jens, here it is without the sysfs stuff Thanks, pulled into for-3.8/drivers -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
Jens, here it is without the sysfs stuff
The following changes since commit ccae7868b0c5697508a541c531cf96b361d62c1c:
drbd: log request sector offset and size for IO errors (2012-10-30 08:39:18
+0100)
are available in the git repository at:
git://git.drbd.org/linux-drbd.git for-jens_drbd-8.4.2
for you to fetch changes up to f1d6a328bbe63b528721a25251ad8f5f1e997804:
drbd: use copy_highpage (2012-11-09 14:22:26 +0100)
Akinobu Mita (1):
drbd: use copy_highpage
Andreas Gruenbacher (210):
drbd: Get rid of req_validator_fn typedef
drbd: Remove superfluous declaration
drbd: Consistently use block_id == ID_SYNCER for checksum based resync
and online verify
drbd: Get rid of BE_DRBD_MAGIC and BE_DRBD_MAGIC_BIG
drbd: Endianness convert the constants instead of the variables
drbd: Magic reserved block_id value cleanup
drbd: Move drbd_free_tl_hash() to drbd_main()
drbd: Update outdated comment
drbd: Request lookup code cleanup (1)
drbd: Request lookup code cleanup (2)
drbd: Request lookup code cleanup (3)
drbd: Request lookup code cleanup (4)
drbd: Add interval tree data structure
drbd: Put sector and size in struct drbd_request into struct drbd_interval
drbd: Use interval tree for overlapping write request detection
drbd: Add read_requests tree
drbd: Use the read and write request trees for request lookups
drbd: Put sector and size in struct drbd_epoch_entry into struct
drbd_interval
drbd: Use interval tree for overlapping epoch entry detection
drbd: Remove the unused hash tables
drbd: Convert all constants in enum drbd_req_event to upper case
drbd: Convert all constants in enum drbd_thread_state to upper case
drbd: Replace the ERR_IF macro with an assert-like macro
drbd: Remove some useless paranoia code
drbd: Inline function overlaps() is now unused
drbd: Interval tree bugfix
drbd: Move cmdname() out of drbd_int.h
drbd: Rename "enum drbd_packets" to "enum drbd_packet"
drbd: Remove redundant initialization
drbd: Initialize the sequence number sent over the network even when not
used
drbd: Move sequence number logic into drbd_receiver.c and simplify it
drbd: Move some functions to where they are used
drbd: struct drbd_request: Introduce a new collision flag
drbd: Remove redundant check from drbd_contains_interval()
drbd: Allow to wait for the completion of an epoch entry as well
drbd: _req_conflicts(): Get rid of the epoch_entries tree
drbd: Remove unnecessary reference counting left-over
drbd: Defer new writes when detecting conflicting writes
drbd: Make the peer_seq updating code more obvious
drbd: Improve the drbd_find_overlap() documentation
drbd: Remove unused variable in struct drbd_conf
drbd: Rename struct drbd_epoch_entry to struct drbd_peer_request
drbd: Clean up some left-overs
drbd: Update some comments
drbd: Local variable renames: e -> peer_req
drbd: Rename drbd_submit_ee -> drbd_submit_peer_request
drbd: Rename drbd_endio_{pri,sec} -> drbd_{,peer_}request_endio
drbd: Iterate over all overlapping intervals in a tree
drbd: Remove obsolete comment
drbd: Use the IS_ALIGNED() macro in some more places
drbd: Use container_of() instead of casting
drbd: Concurrent write detection fix
drbd: Replace atomic_add_return with atomic_inc_return
drbd: Use ping-timeout when waiting for missing ack packets
drbd: Improve how conflicting writes are handled
drbd: Remove redundant check
drbd: Get rid of P_MAX_CMD
drbd: Replace get_asender_cmd() with its implementation
drbd: Remove left-over function prototypes
drbd: drbd_send(): Return a "real" error code if we have no socket
drbd: drbd_get_data_sock(): Return 0 upon success and an error code
otherwise
drbd: Add drbd_send_all(): Send an entire buffer
drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise
drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise
drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise
drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise
drbd: Get rid of USE_DATA_SOCKET and USE_META_SOCKET
drbd: drbd_send_cmd(): Return 0 upon success and an error code otherwise
drbd: drbd_send_sync_param(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_state(): Return 0 upon success and an error code otherwise
drbd: drbd_send_handshake(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_protocol(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_uuids() and its variants: Return 0 upon success and an
error code
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 13:42, Philipp Reisner wrote: > [...] >>> It has the sysfs bits in again. The reason for that is that we want to >>> expose more information by that, and remove the /proc/drbd with the >>> next evolutionary step. -- In case this is a show stopper, let me >>> remove the sysfs bits. >> >> The exact same sysfs bits I complained about last time? If yes, then I >> don't understand why you haven't changed yet. Or why you are trying to >> push the same bits again that got rejected last time. >> > > I had the impression it was rejected because I submitted the pull request > too late to you. In the sense of, it might go in, if it gets submitted > for inclusion before the merge window opens... > Apparently my impression was wrong. You will get an updated pull-request > with the sysfs bits removed It was late, but that was a different issue. I just re-read the Oct 3rd emails on this, and I definitely did recommend that you looked at debugfs or similar for this. We're not putting another top-queue level directory in for a block device that is specific to drbd. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
[...] > > It has the sysfs bits in again. The reason for that is that we want to > > expose more information by that, and remove the /proc/drbd with the > > next evolutionary step. -- In case this is a show stopper, let me > > remove the sysfs bits. > > The exact same sysfs bits I complained about last time? If yes, then I > don't understand why you haven't changed yet. Or why you are trying to > push the same bits again that got rejected last time. > I had the impression it was rejected because I submitted the pull request too late to you. In the sense of, it might go in, if it gets submitted for inclusion before the merge window opens... Apparently my impression was wrong. You will get an updated pull-request with the sysfs bits removed > > Here is the git-pull-request test: > > (The patch subjects removed to make the mail more digestible) > > Please don't do that, it basically makes the pull request useless! A few > hundred extra lines is not an issue. Ok. I intend to send the updated pull-request later today. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 12:36, Philipp Reisner wrote: > Hi Jens, > > Please pull drbd-8.4.2 into your for-3.8/drivers branch. > > The most noticeable change is the support for multiple replicated > volumes in a single DRBD connection. > > This release requires new drbd userland tools >= 8.4.0 (available > since July 2011). > 8.4.x is network protocol compatible with all previous releaes. > This release brings a new meta-data format. Forward (8.3 -> 8.4) > conversion happens complete seamless. Backward conversion > is done by a single command (drbdadm apply-al res). > > The "recent changes" chapter of our user's guide describes all > changes in detail: > http://www.drbd.org/users-guide/ap-recent-changes.html > > Changelog > > 8.4.2 (api:genl1/proto:86-100) > > * Go into inconsistent disk state with on-io-error=pass-on policy > * Timeouts for requests processing on the peer (previously that >worked only if the data socket was congested) > * Conflicting write detection is now based on an interval tree, >removed the hash-tables (necessary for the unlimited BIO sizes) > * Support for multiple volumes (minors, block devices) per connection; >up to 65536 volumes per connection supported > * Reduced IO latencies during some state changes (esp. start resync) > * New on disk format for the AL: double capacity; 4k aligned IO; same space > * Multiple AL changes in a single transaction (precondition for >unlimited BIO sizes) > * DRBD no longer imposes any limit on BIO sizes > * Removed DRBD's limits on the number of minor devices > * DRBD's minors can now be removed (not only unconfigured) > * Switched the user space interface form connector to generic netlink > * The wire-protocol is now a regular connection option, which can be >changed while the device is online > * IO freezing/thawing is done on connection (all volumes) level > * fencing is done on connection (all volumes) level > * Enforce application of activity log after primary crash in user space > * New default values (compared to drbd-8.3) for: minor-count, ko-count, > al-extents, >c-plan-ahead, c-fill-target, c-min-rate, use-rle, on-io-error > * Optional load balancing for read requests: new keyword "read-balance" > * New option 'al-updates no' to disable writing transactions into the >activity log. It is use full if you prefer a full sync after a primary >crash, for improved performance of a spread out random write work load > * Expose the data generation identifies via sysfs > > > Jens, regarding the code: > > It has the sysfs bits in again. The reason for that is that we want to > expose more information by that, and remove the /proc/drbd with the > next evolutionary step. -- In case this is a show stopper, let me > remove the sysfs bits. The exact same sysfs bits I complained about last time? If yes, then I don't understand why you haven't changed yet. Or why you are trying to push the same bits again that got rejected last time. > Here is the git-pull-request test: > (The patch subjects removed to make the mail more digestible) Please don't do that, it basically makes the pull request useless! A few hundred extra lines is not an issue. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 12:36, Philipp Reisner wrote: Hi Jens, Please pull drbd-8.4.2 into your for-3.8/drivers branch. The most noticeable change is the support for multiple replicated volumes in a single DRBD connection. This release requires new drbd userland tools = 8.4.0 (available since July 2011). 8.4.x is network protocol compatible with all previous releaes. This release brings a new meta-data format. Forward (8.3 - 8.4) conversion happens complete seamless. Backward conversion is done by a single command (drbdadm apply-al res). The recent changes chapter of our user's guide describes all changes in detail: http://www.drbd.org/users-guide/ap-recent-changes.html Changelog 8.4.2 (api:genl1/proto:86-100) * Go into inconsistent disk state with on-io-error=pass-on policy * Timeouts for requests processing on the peer (previously that worked only if the data socket was congested) * Conflicting write detection is now based on an interval tree, removed the hash-tables (necessary for the unlimited BIO sizes) * Support for multiple volumes (minors, block devices) per connection; up to 65536 volumes per connection supported * Reduced IO latencies during some state changes (esp. start resync) * New on disk format for the AL: double capacity; 4k aligned IO; same space * Multiple AL changes in a single transaction (precondition for unlimited BIO sizes) * DRBD no longer imposes any limit on BIO sizes * Removed DRBD's limits on the number of minor devices * DRBD's minors can now be removed (not only unconfigured) * Switched the user space interface form connector to generic netlink * The wire-protocol is now a regular connection option, which can be changed while the device is online * IO freezing/thawing is done on connection (all volumes) level * fencing is done on connection (all volumes) level * Enforce application of activity log after primary crash in user space * New default values (compared to drbd-8.3) for: minor-count, ko-count, al-extents, c-plan-ahead, c-fill-target, c-min-rate, use-rle, on-io-error * Optional load balancing for read requests: new keyword read-balance * New option 'al-updates no' to disable writing transactions into the activity log. It is use full if you prefer a full sync after a primary crash, for improved performance of a spread out random write work load * Expose the data generation identifies via sysfs Jens, regarding the code: It has the sysfs bits in again. The reason for that is that we want to expose more information by that, and remove the /proc/drbd with the next evolutionary step. -- In case this is a show stopper, let me remove the sysfs bits. The exact same sysfs bits I complained about last time? If yes, then I don't understand why you haven't changed yet. Or why you are trying to push the same bits again that got rejected last time. Here is the git-pull-request test: (The patch subjects removed to make the mail more digestible) Please don't do that, it basically makes the pull request useless! A few hundred extra lines is not an issue. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
[...] It has the sysfs bits in again. The reason for that is that we want to expose more information by that, and remove the /proc/drbd with the next evolutionary step. -- In case this is a show stopper, let me remove the sysfs bits. The exact same sysfs bits I complained about last time? If yes, then I don't understand why you haven't changed yet. Or why you are trying to push the same bits again that got rejected last time. I had the impression it was rejected because I submitted the pull request too late to you. In the sense of, it might go in, if it gets submitted for inclusion before the merge window opens... Apparently my impression was wrong. You will get an updated pull-request with the sysfs bits removed Here is the git-pull-request test: (The patch subjects removed to make the mail more digestible) Please don't do that, it basically makes the pull request useless! A few hundred extra lines is not an issue. Ok. I intend to send the updated pull-request later today. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 13:42, Philipp Reisner wrote: [...] It has the sysfs bits in again. The reason for that is that we want to expose more information by that, and remove the /proc/drbd with the next evolutionary step. -- In case this is a show stopper, let me remove the sysfs bits. The exact same sysfs bits I complained about last time? If yes, then I don't understand why you haven't changed yet. Or why you are trying to push the same bits again that got rejected last time. I had the impression it was rejected because I submitted the pull request too late to you. In the sense of, it might go in, if it gets submitted for inclusion before the merge window opens... Apparently my impression was wrong. You will get an updated pull-request with the sysfs bits removed It was late, but that was a different issue. I just re-read the Oct 3rd emails on this, and I definitely did recommend that you looked at debugfs or similar for this. We're not putting another top-queue level directory in for a block device that is specific to drbd. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
Jens, here it is without the sysfs stuff
The following changes since commit ccae7868b0c5697508a541c531cf96b361d62c1c:
drbd: log request sector offset and size for IO errors (2012-10-30 08:39:18
+0100)
are available in the git repository at:
git://git.drbd.org/linux-drbd.git for-jens_drbd-8.4.2
for you to fetch changes up to f1d6a328bbe63b528721a25251ad8f5f1e997804:
drbd: use copy_highpage (2012-11-09 14:22:26 +0100)
Akinobu Mita (1):
drbd: use copy_highpage
Andreas Gruenbacher (210):
drbd: Get rid of req_validator_fn typedef
drbd: Remove superfluous declaration
drbd: Consistently use block_id == ID_SYNCER for checksum based resync
and online verify
drbd: Get rid of BE_DRBD_MAGIC and BE_DRBD_MAGIC_BIG
drbd: Endianness convert the constants instead of the variables
drbd: Magic reserved block_id value cleanup
drbd: Move drbd_free_tl_hash() to drbd_main()
drbd: Update outdated comment
drbd: Request lookup code cleanup (1)
drbd: Request lookup code cleanup (2)
drbd: Request lookup code cleanup (3)
drbd: Request lookup code cleanup (4)
drbd: Add interval tree data structure
drbd: Put sector and size in struct drbd_request into struct drbd_interval
drbd: Use interval tree for overlapping write request detection
drbd: Add read_requests tree
drbd: Use the read and write request trees for request lookups
drbd: Put sector and size in struct drbd_epoch_entry into struct
drbd_interval
drbd: Use interval tree for overlapping epoch entry detection
drbd: Remove the unused hash tables
drbd: Convert all constants in enum drbd_req_event to upper case
drbd: Convert all constants in enum drbd_thread_state to upper case
drbd: Replace the ERR_IF macro with an assert-like macro
drbd: Remove some useless paranoia code
drbd: Inline function overlaps() is now unused
drbd: Interval tree bugfix
drbd: Move cmdname() out of drbd_int.h
drbd: Rename enum drbd_packets to enum drbd_packet
drbd: Remove redundant initialization
drbd: Initialize the sequence number sent over the network even when not
used
drbd: Move sequence number logic into drbd_receiver.c and simplify it
drbd: Move some functions to where they are used
drbd: struct drbd_request: Introduce a new collision flag
drbd: Remove redundant check from drbd_contains_interval()
drbd: Allow to wait for the completion of an epoch entry as well
drbd: _req_conflicts(): Get rid of the epoch_entries tree
drbd: Remove unnecessary reference counting left-over
drbd: Defer new writes when detecting conflicting writes
drbd: Make the peer_seq updating code more obvious
drbd: Improve the drbd_find_overlap() documentation
drbd: Remove unused variable in struct drbd_conf
drbd: Rename struct drbd_epoch_entry to struct drbd_peer_request
drbd: Clean up some left-overs
drbd: Update some comments
drbd: Local variable renames: e - peer_req
drbd: Rename drbd_submit_ee - drbd_submit_peer_request
drbd: Rename drbd_endio_{pri,sec} - drbd_{,peer_}request_endio
drbd: Iterate over all overlapping intervals in a tree
drbd: Remove obsolete comment
drbd: Use the IS_ALIGNED() macro in some more places
drbd: Use container_of() instead of casting
drbd: Concurrent write detection fix
drbd: Replace atomic_add_return with atomic_inc_return
drbd: Use ping-timeout when waiting for missing ack packets
drbd: Improve how conflicting writes are handled
drbd: Remove redundant check
drbd: Get rid of P_MAX_CMD
drbd: Replace get_asender_cmd() with its implementation
drbd: Remove left-over function prototypes
drbd: drbd_send(): Return a real error code if we have no socket
drbd: drbd_get_data_sock(): Return 0 upon success and an error code
otherwise
drbd: Add drbd_send_all(): Send an entire buffer
drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise
drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise
drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise
drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise
drbd: Get rid of USE_DATA_SOCKET and USE_META_SOCKET
drbd: drbd_send_cmd(): Return 0 upon success and an error code otherwise
drbd: drbd_send_sync_param(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_state(): Return 0 upon success and an error code otherwise
drbd: drbd_send_handshake(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_protocol(): Return 0 upon success and an error code
otherwise
drbd: drbd_send_uuids() and its variants: Return 0 upon success and an
error code otherwise
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 14:33, Philipp Reisner wrote: Jens, here it is without the sysfs stuff Thanks, pulled into for-3.8/drivers -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 15:18, Jens Axboe wrote: On 2012-11-09 14:33, Philipp Reisner wrote: Jens, here it is without the sysfs stuff Thanks, pulled into for-3.8/drivers I didn't say anything, but I've been fuming a bit the last few series of merge windows. You need to stop these insanely massive pull requests. I've been large since this is just a driver, but it can't continue. We should have reached stability a long time ago. Your pull requests contain a shit load of items, are you guys paying per commit? Look at these: drbd: Request lookup code cleanup (1) drbd: Request lookup code cleanup (2) drbd: Request lookup code cleanup (3) drbd: Request lookup code cleanup (4) or drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise along with FIFTY or so more of these. WTF is this? drbd: Converted helper functions for drbd_send() to tconn drbd: Converted drbd_send() from mdev to tconn drbd: Converted drbd_send_fp() from mdev to tconn ... I don't think I need to go on. So from now on, to get items into the kernel, what you will do is: - Stop doing insane commits like the above. It just doesn't make sense. - Send pull requests in a timely fashion. No more of this lets collect ALL the things then send it off. Collect small bug fixes, send those off. Develop some feature or make some changes, send that off. Etc. The fact that your initial pull request had to MASK all these commits should have rung big bells in your head. It's a clear sign of a huge problem in your development model. If you can't clean this up, then it's not going in. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
Am Freitag, 9. November 2012, 15:50:24 schrieb Jens Axboe: On 2012-11-09 15:18, Jens Axboe wrote: On 2012-11-09 14:33, Philipp Reisner wrote: Jens, here it is without the sysfs stuff Thanks, pulled into for-3.8/drivers I didn't say anything, but I've been fuming a bit the last few series of merge windows. You need to stop these insanely massive pull requests. I've been large since this is just a driver, but it can't continue. We should have reached stability a long time ago. Your pull requests contain a shit load of items, are you guys paying per commit? Look at these: drbd: Request lookup code cleanup (1) drbd: Request lookup code cleanup (2) drbd: Request lookup code cleanup (3) drbd: Request lookup code cleanup (4) We are living there in the belief that we should break up big changes in review able chunks or drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise Function by function gets converted to the return 0 upon success call semantics. Do you prefer that all of that should be done in a single commit? along with FIFTY or so more of these. WTF is this? drbd: Converted helper functions for drbd_send() to tconn drbd: Converted drbd_send() from mdev to tconn drbd: Converted drbd_send_fp() from mdev to tconn Should it instead be in a single commit? I don't think I need to go on. So from now on, to get items into the kernel, what you will do is: - Stop doing insane commits like the above. It just doesn't make sense. - Send pull requests in a timely fashion. No more of this lets collect ALL the things then send it off. Collect small bug fixes, send those off. Develop some feature or make some changes, send that off. Etc. That works well for individual features, and we have been doing that for the last two Years. But at this time we changed the object model. In the old code we had a single kind of DRBD-in-kernel-object: a resource Now we have two kinds: resources and volumes. 8.3: a resource had a single implicit volume 8.4: a resource might contain multiple volumes, each volume belongs to a single resource. In the next ~12 month you will get only small features/updates etc... for the 8.4 code base. The fact that your initial pull request had to MASK all these commits should have rung big bells in your head. It's a clear sign of a huge problem in your development model. If you can't clean this up, then it's not going in. Fundamental changes in our object model require such huge change sets. Jens, we will not stop where we are today. We plan to introduce a new object: a connection. (The ability to mirror for one machine to *multiple* receivers.) Is it a better fit to introduce it then as a new driver? E.g. called it drbd9. Should it use a new major number? Best, Phil -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] drbd-8.4.2 for the linux-3.8 merge window
On 2012-11-09 16:19, Philipp Reisner wrote: Am Freitag, 9. November 2012, 15:50:24 schrieb Jens Axboe: On 2012-11-09 15:18, Jens Axboe wrote: On 2012-11-09 14:33, Philipp Reisner wrote: Jens, here it is without the sysfs stuff Thanks, pulled into for-3.8/drivers I didn't say anything, but I've been fuming a bit the last few series of merge windows. You need to stop these insanely massive pull requests. I've been large since this is just a driver, but it can't continue. We should have reached stability a long time ago. Your pull requests contain a shit load of items, are you guys paying per commit? Look at these: drbd: Request lookup code cleanup (1) drbd: Request lookup code cleanup (2) drbd: Request lookup code cleanup (3) drbd: Request lookup code cleanup (4) We are living there in the belief that we should break up big changes in review able chunks Don't be silly. Of course things should be broken up. But they should be broken up in logical steps needed to convert from state X to Y. What does reqeust code cleanup (1) mean? Nothing. Lets take a look at the commit message: Move _ar_id_to_req() to drbd_receiver.c and mark it non-inline. Remove the leading underscores from _ar_id_to_req() and _ack_id_to_req(). Mark ar_hash_slot() inline. which is exactly the wrong way as well. A commit message should explain why a given change is done. The above describes how it's done. We can all read the code and see that. In other words, you need to school people on what a commit is, and what a series of commits is. or drbd: conn_send_cmd2(): Return 0 upon success and an error code otherwise drbd: _conn_send_cmd(): Return 0 upon success and an error code otherwise drbd: _drbd_send_cmd(): Return 0 upon success and an error code otherwise drbd: conn_send_cmd(): Return 0 upon success and an error code otherwise Function by function gets converted to the return 0 upon success call semantics. Do you prefer that all of that should be done in a single commit? For this case, yes indeed. Then it'd make more sense to describe why that change was made. For this particular one, it's fairly easy to guess why the change was needed. along with FIFTY or so more of these. WTF is this? drbd: Converted helper functions for drbd_send() to tconn drbd: Converted drbd_send() from mdev to tconn drbd: Converted drbd_send_fp() from mdev to tconn Should it instead be in a single commit? I don't think I need to go on. So from now on, to get items into the kernel, what you will do is: - Stop doing insane commits like the above. It just doesn't make sense. - Send pull requests in a timely fashion. No more of this lets collect ALL the things then send it off. Collect small bug fixes, send those off. Develop some feature or make some changes, send that off. Etc. That works well for individual features, and we have been doing that for the last two Years. But at this time we changed the object model. In the old code we had a single kind of DRBD-in-kernel-object: a resource Now we have two kinds: resources and volumes. 8.3: a resource had a single implicit volume 8.4: a resource might contain multiple volumes, each volume belongs to a single resource. In the next ~12 month you will get only small features/updates etc... for the 8.4 code base. If you had just structured it a bit better, then 8.3 - 8.4 transition would look a lot less painful. As it stands right now, it's just a huge mess of little changes. But I'm happy that you think that the next 12 months will have only gradual changes for bug fixes and features. The fact that your initial pull request had to MASK all these commits should have rung big bells in your head. It's a clear sign of a huge problem in your development model. If you can't clean this up, then it's not going in. Fundamental changes in our object model require such huge change sets. Jens, we will not stop where we are today. We plan to introduce a new object: a connection. (The ability to mirror for one machine to *multiple* receivers.) If you think I'm asking you to stop developing, then you are completely missing the message. I'm asking you to CHANGE how you develop, or at least present, these changes. Tons of little changes reak of hey lets do this in this function. then lets also do this in that function. A bit more structure would go a long way. What I usually do is that I may develop from the first model, then refactor and structure everything to look a lot nicer. A big part of being able to review a patch series is understanding where it's going. Step 1 does this, step 2 does that, etc. If you have 500 changes of change return code from void to err/0, well... It may be that you have nice structure underneath it and it's just hidden behind this mess. If so, it'll be easier for you to make it better. Is it a better fit to introduce it then as a new
