1. 02 May, 2013 40 commits
    • Alex Elder's avatar
      libceph: kill message trail · 9d2a06c2
      Alex Elder authored
      The wart that is the ceph message trail can now be removed, because
      its only user was the osd client, and the previous patch made that
      no longer the case.
      
      The result allows write_partial_msg_pages() to be simplified
      considerably.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      9d2a06c2
    • Alex Elder's avatar
      libceph: kill osd request r_trail · 95e072eb
      Alex Elder authored
      The osd trail is a pagelist, used only for a CALL osd operation
      to hold the class and method names, along with any input data for
      the call.
      
      It is only currently used by the rbd client, and when it's used it
      is the only bit of outbound data in the osd request.  Since we
      already support (non-trail) pagelist data in a message, we can
      just save this outbound CALL data in the "normal" pagelist rather
      than the trail, and get rid of the trail entirely.
      
      The existing pagelist support depends on the pagelist being
      dynamically allocated, and ownership of it is passed to the
      messenger once it's been attached to a message.  (That is to say,
      the messenger releases and frees the pagelist when it's done with
      it).  That means we need to dynamically allocate the pagelist also.
      
      Note that we simply assert that the allocation of a pagelist
      structure succeeds.  Appending to a pagelist might require a dynamic
      allocation, so we're already assuming we won't run into trouble
      doing so (we're just ignore any failures--and that should be fixed
      at some point).
      
      This resolves:
          http://tracker.ceph.com/issues/4407Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      95e072eb
    • Alex Elder's avatar
      libceph: have osd requests support pagelist data · 9a5e6d09
      Alex Elder authored
      Add support for recording a ceph pagelist as data associated with an
      osd request.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      9a5e6d09
    • Alex Elder's avatar
      libceph: let osd ops determine request data length · 175face2
      Alex Elder authored
      The length of outgoing data in an osd request is dependent on the
      osd ops that are embedded in that request.  Each op is encoded into
      a request message using osd_req_encode_op(), so that should be used
      to determine the amount of outgoing data implied by the op as it
      is encoded.
      
      Have osd_req_encode_op() return the number of bytes of outgoing data
      implied by the op being encoded, and accumulate and use that in
      ceph_osdc_build_request().
      
      As a result, ceph_osdc_build_request() no longer requires its "len"
      parameter, so get rid of it.
      
      Using the sum of the op lengths rather than the length provided is
      a valid change because:
          - The only callers of osd ceph_osdc_build_request() are
            rbd and the osd client (in ceph_osdc_new_request() on
            behalf of the file system).
          - When rbd calls it, the length provided is only non-zero for
            write requests, and in that case the single op has the
            same length value as what was passed here.
          - When called from ceph_osdc_new_request(), (it's not all that
            easy to see, but) the length passed is also always the same
            as the extent length encoded in its (single) write op if
            present.
      
      This resolves:
          http://tracker.ceph.com/issues/4406Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      175face2
    • Alex Elder's avatar
      libceph: implement pages array cursor · e766d7b5
      Alex Elder authored
      Implement and use cursor routines for page array message data items
      for outbound message data.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      e766d7b5
    • Alex Elder's avatar
      libceph: implement bio message data item cursor · 6aaa4511
      Alex Elder authored
      Implement and use cursor routines for bio message data items for
      outbound message data.
      
      (See the previous commit for reasoning in support of the changes
      in out_msg_pos_next().)
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      6aaa4511
    • Alex Elder's avatar
      libceph: use data cursor for message pagelist · 7fe1e5e5
      Alex Elder authored
      Switch to using the message cursor for the (non-trail) outgoing
      pagelist data item in a message if present.
      
      Notes on the logic changes in out_msg_pos_next():
          - only the mds client uses a ceph pagelist for message data;
          - if the mds client ever uses a pagelist, it never uses a page
            array (or anything else, for that matter) for data in the same
            message;
          - only the osd client uses the trail portion of a message data,
            and when it does, it never uses any other data fields for
            outgoing data in the same message; and finally
          - only the rbd client uses bio message data (never pagelist).
      
      Therefore out_msg_pos_next() can assume:
          - if we're in the trail portion of a message, the message data
            pagelist, data, and bio can be ignored; and
          - if there is a page list, there will never be any a bio or page
            array data, and vice-versa.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      7fe1e5e5
    • Alex Elder's avatar
      libceph: prepare for other message data item types · dd236fcb
      Alex Elder authored
      This just inserts some infrastructure in preparation for handling
      other types of ceph message data items.  No functional changes,
      just trying to simplify review by separating out some noise.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      dd236fcb
    • Alex Elder's avatar
      libceph: start defining message data cursor · fe38a2b6
      Alex Elder authored
      This patch lays out the foundation for using generic routines to
      manage processing items of message data.
      
      For simplicity, we'll start with just the trail portion of a
      message, because it stands alone and is only present for outgoing
      data.
      
      First some basic concepts.  We'll use the term "data item" to
      represent one of the ceph_msg_data structures associated with a
      message.  There are currently four of those, with single-letter
      field names p, l, b, and t.  A data item is further broken into
      "pieces" which always lie in a single page.  A data item will
      include a "cursor" that will track state as the memory defined by
      the item is consumed by sending data from or receiving data into it.
      
      We define three routines to manipulate a data item's cursor: the
      "init" routine; the "next" routine; and the "advance" routine.  The
      "init" routine initializes the cursor so it points at the beginning
      of the first piece in the item.  The "next" routine returns the
      page, page offset, and length (limited by both the page and item
      size) of the next unconsumed piece in the item.  It also indicates
      to the caller whether the piece being returned is the last one in
      the data item.
      
      The "advance" routine consumes the requested number of bytes in the
      item (advancing the cursor).  This is used to record the number of
      bytes from the current piece that were actually sent or received by
      the network code.  It returns an indication of whether the result
      means the current piece has been fully consumed.  This is used by
      the message send code to determine whether it should calculate the
      CRC for the next piece processed.
      
      The trail of a message is implemented as a ceph pagelist.  The
      routines defined for it will be usable for non-trail pagelist data
      as well.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      fe38a2b6
    • Alex Elder's avatar
      libceph: abstract message data · 43794509
      Alex Elder authored
      Group the types of message data into an abstract structure with a
      type indicator and a union containing fields appropriate to the
      type of data it represents.  Use this to represent the pages,
      pagelist, bio, and trail in a ceph message.
      
      Verify message data is of type NONE in ceph_msg_data_set_*()
      routines.  Since information about message data of type NONE really
      should not be interpreted, get rid of the other assertions in those
      functions.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      43794509
    • Alex Elder's avatar
      libceph: be explicit about message data representation · f9e15777
      Alex Elder authored
      A ceph message has a data payload portion.  The memory for that data
      (either the source of data to send or the location to place data
      that is received) is specified in several ways.  The ceph_msg
      structure includes fields for all of those ways, but this
      mispresents the fact that not all of them are used at a time.
      
      Specifically, the data in a message can be in:
          - an array of pages
          - a list of pages
          - a list of Linux bios
          - a second list of pages (the "trail")
      (The two page lists are currently only ever used for outgoing data.)
      
      Impose more structure on the ceph message, making the grouping of
      some of these fields explicit.  Shorten the name of the
      "page_alignment" field.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      f9e15777
    • Alex Elder's avatar
      libceph: define ceph_msg_has_*() data macros · 97fb1c7f
      Alex Elder authored
      Define and use macros ceph_msg_has_*() to determine whether to
      operate on the pages, pagelist, bio, and trail fields of a message.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      97fb1c7f
    • Alex Elder's avatar
      libceph: define and use ceph_crc32c_page() · 35b62808
      Alex Elder authored
      Factor out a common block of code that updates a CRC calculation
      over a range of data in a page.
      
      This and the preceding patches are related to:
          http://tracker.ceph.com/issues/4403Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      35b62808
    • Alex Elder's avatar
      libceph: define and use ceph_tcp_recvpage() · afb3d90e
      Alex Elder authored
      Define a new function ceph_tcp_recvpage() that behaves in a way
      comparable to ceph_tcp_sendpage().
      
      Rearrange the code in both read_partial_message_pages() and
      read_partial_message_bio() so they have matching structure,
      (similar to what's in write_partial_msg_pages()), and use
      this new function.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      afb3d90e
    • Alex Elder's avatar
      libceph: encapsulate reading message data · 34d2d200
      Alex Elder authored
      Pull the code that reads the data portion into a message into
      a separate function read_partial_msg_data().
      
      Rename write_partial_msg_pages() to be write_partial_message_data()
      to match its read counterpart, and to reflect its more generic
      purpose.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      34d2d200
    • Alex Elder's avatar
      libceph: small write_partial_msg_pages() refactor · e387d525
      Alex Elder authored
      Define local variables page_offset and length to represent the range
      of bytes within a page that will be sent by ceph_tcp_sendpage() in
      write_partial_msg_pages().
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      e387d525
    • Alex Elder's avatar
      libceph: consolidate message prep code · 78625051
      Alex Elder authored
      In prepare_write_message_data(), various fields are initialized in
      preparation for writing message data out.  Meanwhile, in
      read_partial_message(), there is essentially the same block of code,
      operating on message variables associated with an incoming message.
      
      Generalize prepare_write_message_data() so it works for both
      incoming and outcoming messages, and use it in both spots.  The
      did_page_crc is not used for input (so it's harmless to initialize
      it).
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      78625051
    • Alex Elder's avatar
      libceph: use local variables for message positions · bae6acd9
      Alex Elder authored
      There are several places where a message's out_msg_pos or in_msg_pos
      field is used repeatedly within a function.  Use a local pointer
      variable for this purpose to unclutter the code.
      
      This and the upcoming cleanup patches are related to:
          http://tracker.ceph.com/issues/4403Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      bae6acd9
    • Alex Elder's avatar
      libceph: don't clear bio_iter in prepare_write_message() · 98a03708
      Alex Elder authored
      At one time it was necessary to clear a message's bio_iter field to
      avoid a bad pointer dereference in write_partial_msg_pages().
      
      That no longer seems to be the case.  Here's why.
      
      The message's bio fields represent (in this case) outgoing data.
      Between where the bio_iter is made NULL in prepare_write_message()
      and the call in that function to prepare_message_data(), the
      bio fields are never used.
      
      In prepare_message_data(), init-bio_iter() is called, and the result
      of that overwrites the value in the message's bio_iter field.
      
      Because it gets overwritten anyway, there is no need to set it to
      NULL.  So don't do it.
      
      This resolves:
          http://tracker.ceph.com/issues/4402Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      98a03708
    • Alex Elder's avatar
      libceph: activate message data assignment checks · 07aa1558
      Alex Elder authored
      The mds client no longer tries to assign zero-length message data,
      and the osd client no longer sets its data info more than once.
      This allows us to activate assertions in the messenger to verify
      these things never happen.
      
      This resolves both of these:
          http://tracker.ceph.com/issues/4263
          http://tracker.ceph.com/issues/4284Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarGreg Farnum <greg@inktank.com>
      07aa1558
    • Alex Elder's avatar
      libceph: set response data fields earlier · 70636773
      Alex Elder authored
      When an incoming message is destined for the osd client, the
      messenger calls the osd client's alloc_msg method.  That function
      looks up which request has the tid matching the incoming message,
      and returns the request message that was preallocated to receive the
      response.  The response message is therefore known before the
      request is even started.
      
      Between the start of the request and the receipt of the response,
      the request and its data fields will not change, so there's no
      reason we need to hold off setting them.  In fact it's preferable
      to set them just once because it's more obvious that they're
      unchanging.
      
      So set up the fields describing where incoming data is to land in a
      response message at the beginning of ceph_osdc_start_request().
      Define a helper function that sets these fields, and use it to
      set the fields for both outgoing data in the request message and
      incoming data in the response.
      
      This resolves:
          http://tracker.ceph.com/issues/4284Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      70636773
    • Alex Elder's avatar
      libceph: record message data byte length · 4a73ef27
      Alex Elder authored
      Record the number of bytes of data in a page array rather than the
      number of pages in the array.  It can be assumed that the page array
      is of sufficient size to hold the number of bytes indicated (and
      offset by the indicated alignment).
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      4a73ef27
    • Alex Elder's avatar
      ceph: only set message data pointers if non-empty · ebf18f47
      Alex Elder authored
      Change it so we only assign outgoing data information for messages
      if there is outgoing data to send.
      
      This then allows us to add a few more (currently commented-out)
      assertions.
      
      This is related to:
          http://tracker.ceph.com/issues/4284Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarGreg Farnum <greg@inktank.com>
      ebf18f47
    • Alex Elder's avatar
      libceph: isolate other message data fields · 27fa8385
      Alex Elder authored
      Define ceph_msg_data_set_pagelist(), ceph_msg_data_set_bio(), and
      ceph_msg_data_set_trail() to clearly abstract the assignment of the
      remaining data-related fields in a ceph message structure.  Use the
      new functions in the osd client and mds client.
      
      This partially resolves:
          http://tracker.ceph.com/issues/4263Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      27fa8385
    • Alex Elder's avatar
      libceph: set page info with byte length · f1baeb2b
      Alex Elder authored
      When setting page array information for message data, provide the
      byte length rather than the page count ceph_msg_data_set_pages().
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      f1baeb2b
    • Alex Elder's avatar
      libceph: isolate message page field manipulation · 02afca6c
      Alex Elder authored
      Define a function ceph_msg_data_set_pages(), which more clearly
      abstracts the assignment page-related fields for data in a ceph
      message structure.  Use this new function in the osd client and mds
      client.
      
      Ideally, these fields would never be set more than once (with
      BUG_ON() calls to guarantee that).  At the moment though the osd
      client sets these every time it receives a message, and in the event
      of a communication problem this can happen more than once.  (This
      will be resolved shortly, but setting up these helpers first makes
      it all a bit easier to work with.)
      
      Rearrange the field order in a ceph_msg structure to group those
      that are used to define the possible data payloads.
      
      This partially resolves:
          http://tracker.ceph.com/issues/4263Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      02afca6c
    • Alex Elder's avatar
      libceph: record byte count not page count · e0c59487
      Alex Elder authored
      Record the byte count for an osd request rather than the page count.
      The number of pages can always be derived from the byte count (and
      alignment/offset) but the reverse is not true.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      e0c59487
    • Alex Elder's avatar
      libceph: simplify new message initialization · 9516e45b
      Alex Elder authored
      Rather than explicitly initializing many fields to 0, NULL, or false
      in a newly-allocated message, just use kzalloc() for allocating new
      messages.  This will become a much more convenient way of doing
      things anyway for upcoming patches that abstract the data field.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      9516e45b
    • Alex Elder's avatar
      libceph: advance pagelist with list_rotate_left() · 35c7bfbc
      Alex Elder authored
      While processing an outgoing pagelist (either the data pagelist or
      trail) in a ceph message, the messenger cycles through each of the
      pages on the list.  This is accomplished in out_msg_pos_next(), if
      the end of the first page on the list is reached, the first page is
      moved to the end of the list.
      
      There is a list operation, list_rotate_left(), which performs
      exactly this operation, and by using it, what's really going on
      becomes more obvious.
      
      So replace these two list_move_tail() calls with list_rotate_left().
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      35c7bfbc
    • Alex Elder's avatar
      libceph: define and use in_msg_pos_next() · e788182f
      Alex Elder authored
      Define a new function in_msg_pos_next() to match out_msg_pos_next(),
      and use it in place of code at the end of read_partial_message_pages()
      and read_partial_message_bio().
      
      Note that the page number is incremented and offset reset under
      slightly different conditions from before.  The result is
      equivalent, however, as explained below.
      
      Each time an incoming message is going to arrive, we find out how
      much room is left--not surpassing the current page--and provide that
      as the number of bytes to receive.  So the amount we'll use is the
      lesser of:  all that's left of the entire request; and all that's
      left in the current page.
      
      If we received exactly how many were requested, we either reached
      the end of the request or the end of the page.  In the first case,
      we're done, in the second, we move onto the next page in the array.
      
      In all cases but (possibly) on the last page, after adding the
      number of bytes received, page_pos == PAGE_SIZE.  On the last page,
      it doesn't really matter whether we increment the page number and
      reset the page position, because we're done and we won't come back
      here again.  The code previously skipped over that last case,
      basically.  The new code handles that case the same as the others,
      incrementing and resetting.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      e788182f
    • Alex Elder's avatar
      libceph: kill args in read_partial_message_bio() · b3d56fab
      Alex Elder authored
      There is only one caller for read_partial_message_bio(), and it
      always passes &msg->bio_iter and &bio_seg as the second and third
      arguments.  Furthermore, the message in question is always the
      connection's in_msg, and we can get that inside the called function.
      
      So drop those two parameters and use their derived equivalents.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      b3d56fab
    • Alex Elder's avatar
      libceph: change type of ceph_tcp_sendpage() "more" · e1dcb128
      Alex Elder authored
      Change the type of the "more" parameter from int to bool.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      e1dcb128
    • Alex Elder's avatar
      libceph: minor byte order problems in read_partial_message() · 6ebc8b32
      Alex Elder authored
      Some values printed are not (necessarily) in CPU order.  We already
      have a copy of the converted versions, so use them.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      6ebc8b32
    • Alex Elder's avatar
      libceph: define CEPH_MSG_MAX_MIDDLE_LEN · 7b11ba37
      Alex Elder authored
      This is probably unnecessary but the code read as if it were wrong
      in read_partial_message().
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      7b11ba37
    • Alex Elder's avatar
      libceph: clean up skipped message logic · 4137577a
      Alex Elder authored
      In ceph_con_in_msg_alloc() it is possible for a connection's
      alloc_msg method to indicate an incoming message should be skipped.
      By default, read_partial_message() initializes the skip variable
      to 0 before it gets provided to ceph_con_in_msg_alloc().
      
      The osd client, mon client, and mds client each supply an alloc_msg
      method.  The mds client always assigns skip to be 0.
      
      The other two leave the skip value of as-is, or assigns it to zero,
      except:
          - if no (osd or mon) request having the given tid is found, in
            which case skip is set to 1 and NULL is returned; or
          - in the osd client, if the data of the reply message is not
            adequate to hold the message to be read, it assigns skip
            value 1 and returns NULL.
      So the returned message pointer will always be NULL if skip is ever
      non-zero.
      
      Clean up the logic a bit in ceph_con_in_msg_alloc() to make this
      state of affairs more obvious.  Add a comment explaining how a null
      message pointer can mean either a message that should be skipped or
      a problem allocating a message.
      
      This resolves:
          http://tracker.ceph.com/issues/4324Reported-by: default avatarGreg Farnum <greg@inktank.com>
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarGreg Farnum <greg@inktank.com>
      4137577a
    • Alex Elder's avatar
      libceph: separate read and write data · 0fff87ec
      Alex Elder authored
      An osd request defines information about where data to be read
      should be placed as well as where data to write comes from.
      Currently these are represented by common fields.
      
      Keep information about data for writing separate from data to be
      read by splitting these into data_in and data_out fields.
      
      This is the key patch in this whole series, in that it actually
      identifies which osd requests generate outgoing data and which
      generate incoming data.  It's less obvious (currently) that an osd
      CALL op generates both outgoing and incoming data; that's the focus
      of some upcoming work.
      
      This resolves:
          http://tracker.ceph.com/issues/4127Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      0fff87ec
    • Alex Elder's avatar
      libceph: distinguish page and bio requests · 2ac2b7a6
      Alex Elder authored
      An osd request uses either pages or a bio list for its data.  Use a
      union to record information about the two, and add a data type
      tag to select between them.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      2ac2b7a6
    • Alex Elder's avatar
      libceph: separate osd request data info · 2794a82a
      Alex Elder authored
      Pull the fields in an osd request structure that define the data for
      the request out into a separate structure.
      Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      2794a82a
    • Alex Elder's avatar
      libceph: don't assign page info in ceph_osdc_new_request() · 153e5167
      Alex Elder authored
      Currently ceph_osdc_new_request() assigns an osd request's
      r_num_pages and r_alignment fields.  The only thing it does
      after that is call ceph_osdc_build_request(), and that doesn't
      need those fields to be assigned.
      
      Move the assignment of those fields out of ceph_osdc_new_request()
      and into its caller.  As a result, the page_align parameter is no
      longer used, so get rid of it.
      
      Note that in ceph_sync_write(), the value for req->r_num_pages had
      already been calculated earlier (as num_pages, and fortunately
      it was computed the same way).  So don't bother recomputing it,
      but because it's not needed earlier, move that calculation after the
      call to ceph_osdc_new_request().  Hold off making the assignment to
      r_alignment, doing it instead r_pages and r_num_pages are
      getting set.
      
      Similarly, in start_read(), nr_pages already holds the number of
      pages in the array (and is calculated the same way), so there's no
      need to recompute it.  Move the assignment of the page alignment
      down with the others there as well.
      
      This and the next few patches are preparation work for:
          http://tracker.ceph.com/issues/4127Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      153e5167
    • Alex Elder's avatar
      ceph: simplify ceph_sync_write() page_align calculation · 3a42b6c4
      Alex Elder authored
      (This is being reposted.  The first one had a problem because it
      erroneously added a similar change elsewhere; that change has been
      dropped.)
      
      The next patch in this series points out that the calculation for
      the number of pages in an osd request is getting done twice.  It
      is not obvious, but the result of both calculations is identical.
      This patch simplifies one of them--as a separate step--to make
      it clear that the transformation in the next patch is valid.
      
      In ceph_sync_write() there is some magic that computes page_align
      for an osd request.  But a little analysis shows it can be
      simplified.
      
      First, we have:
       	io_align = pos & ~PAGE_MASK;
      which is used here:
      	page_align = (pos - io_align + buf_align) & ~PAGE_MASK;
      
      Note (pos - io_align) simply rounds "pos" down to the nearest multiple
      of the page size.
      
      We also have:
       	buf_align = (unsigned long)data & ~PAGE_MASK;
      
      Adding buf_align to that rounded-down "pos" value will stay within
      the same page; the result will just be offset by the page offset for
      the "data" pointer.  The final mask therefore leaves just the value
      of "buf_align".
      
      One more simplification.  Note that the result of calc_pages_for()
      is invariant of which page the offset starts in--the only thing that
      matters is the offset within the starting page.  We will have
      put the proper page offset to use into "page_align", so just use
      that in calculating num_pages.
      
      This resolves:
          http://tracker.ceph.com/issues/4166Signed-off-by: default avatarAlex Elder <elder@inktank.com>
      Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
      3a42b6c4